Closed Bug 325005 Opened 19 years ago Closed 19 years ago

Documents parsed as data load subframes and objects

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: bzbarsky, Assigned: bzbarsky)

References

(
URL
)

Details

(Keywords: fixed1.8.1, verified1.8.0.2, Whiteboard: [sg:moderate][rft-dl])

Attachments

(2 files)

Patch
2.79 KB, patch
Details | Diff | Splinter Review
Same as diff -w
2.05 KB, patch
sicking
: review+
jst
: superreview+
jst
: approval-branch-1.8.1+
dveditz
: approval1.8.0.2+
Details | Diff | Splinter Review 
See URL in the URL field.  Should that alert a 1?  Or a 0?

We block loading of images, stylesheets, and scripts in a document loaded as data.  Should we also block loading of subdocuments and objects?  That is, perhaps we should remove the type check at http://lxr.mozilla.org/seamonkey/source/content/base/src/nsDataDocumentContentPolicy.cpp#56 ?

Note that this would affect documents loaded via XMLDocument.load, XMLHttpRequest, and DOMParser.  Do we need to differentiate between these cases in any way here?
Note that loading subframes means that script can execute inside an XMLHttpRequest or DOMParser document, like so:

javascript:void(new DOMParser().parseFromString("<html xmlns='http://www.w3.org/1999/xhtml'><iframe src='data:text/html,&lt;script&gt;alert(1);&lt;/script&gt;'/></html>", "text/xml"))

I'm not sure whether this is a problem, since I'm not sure that this script has any access to the caller document.  But this is still a little troubling.

At the very least, I would think that if we _do_ load subframes in a data document we should load them as data, right?
Flags: blocking1.9a1?
Even simpler:

javascript:void(new DOMParser().parseFromString("<html xmlns='http://www.w3.org/1999/xhtml'><iframe src='javascript:alert(1)'/></html>", "text/xml"))


The script runs with the privileges of the page that used DOMParser, so it can make requests to that site, read cookies for that site, etc.
OK, that seems like a problem.  Sounds to me like we should just block all loads, not just some types.  Any objections?

Also, do we need this fixed on branches?  Doing that on 1.7 could be interesting... :(
Flags: blocking1.8.1?
Flags: blocking1.8.0.2?
Seeing that we have a same-origin policy in place, I'm not sure what the attack is here?

That said, it's of doubtful value, and probably unexpected for many users, to load external document in iframes and such.
Blocks: 325006
Attached patch Patch — — Splinter Review 
Assignee: general → bzbarsky
Status: NEW → ASSIGNED

        Attachment #210059 -
        Flags: superreview?(jst)

        Attachment #210059 -
        Flags: review?(bugmail)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        Same as diff -w
        — 
          — Splinter Review
      

    


  

        Attachment #210060 -
        Flags: superreview?(jst)

        Attachment #210060 -
        Flags: review?(bugmail)

    
  

        Attachment #210059 -
        Flags: superreview?(jst)

        Attachment #210059 -
        Flags: review?(bugmail)

    
  
OS: Linux → All
Hardware: PC → All
Whiteboard: [sg:moderate]

    
    

    
  
Comment on attachment 210060 [details] [diff] [review]
Same as diff -w

sr=jst

        Attachment #210060 -
        Flags: superreview?(jst) → superreview+

    
    

    
  
Comment on attachment 210060 [details] [diff] [review]
Same as diff -w

We should probably take this on the 1.8 branch.  Jesse, do we need a fix for this on the 1.7 branch?

        Attachment #210060 -
        Flags: branch-1.8.1?(jst)

        Attachment #210060 -
        Flags: approval1.8.0.2?

    
    

    
  
Fixed on trunk.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED

        Attachment #210060 -
        Flags: branch-1.8.1?(jst) → branch-1.8.1+

    
  
Keywords: fixed1.8.1
Flags: blocking1.8.1?
Flags: blocking1.8.1+
Flags: blocking1.8.0.2?
Flags: blocking1.8.0.2+
Flags: blocking1.7.13?
Flags: blocking-aviary1.0.8?

    
  
Flags: blocking1.9a1?

    
    

    
  
Comment on attachment 210060 [details] [diff] [review]
Same as diff -w

approved for 1.8.0 branch, a=dveditz for drivers

        Attachment #210060 -
        Flags: approval1.8.0.2? → approval1.8.0.2+

    
    

    
  
Fixed for 1.8.0.2.
Keywords: fixed1.8.0.2

    
    

    
  
Comment on attachment 210060 [details] [diff] [review]
Same as diff -w

I think we want this on the older branches too.

        Attachment #210060 -
        Flags: approval1.7.13?

        Attachment #210060 -
        Flags: approval-aviary1.0.8?
Flags: blocking1.7.14?
Flags: blocking1.7.13?
Flags: blocking-aviary1.0.9?
Flags: blocking-aviary1.0.8?

        Attachment #210060 -
        Flags: approval1.7.13?

        Attachment #210060 -
        Flags: approval-aviary1.0.8?

    
    

    
  
That patch doesn't apply to the older branches, since they have no nsDataDocumentContentPolicy.  We could introduce that on the branches, but that will take some work, since the API is not the same (not the same between aviary and 1.7 branches, and not the same between either and trunk).

I suppose I can do that if desired.  :(
Whiteboard: [sg:moderate] → [sg:moderate][rft-dl]

    
    

    
  
v.fixed on 1.8.0 branch with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.2) Gecko/20060308 Firefox/1.5.0.2, bz's simple testcase in comment #2. 

Nothing happens... no alert, nothing in jsc, which is expected, right?  We simply reject the doc load and the js inside the iframe?
URL: "text/xml").getElementsByTagName("ifr... → javascript:void(new DOMParser().parse...
Keywords: fixed1.8.0.2 → verified1.8.0.2

    
    

    
  
Per Dveditz's request, I checked to see that this was still fixed for 1.8.1.12. It is.

Checked with  Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.12) Gecko/2008013015 Firefox/2.0.0.12
Component: DOM → DOM: Core & HTML

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: