Closed Bug 466937 (CVE-2009-0355) Opened 16 years ago Closed 16 years ago

File stealing with SessionStore

Categories

(Firefox :: Session Restore, defect)

Product:
Firefox
Component:
Session Restore
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Firefox 3.6a1

People

(Reporter: moz_bug_r_a4, Assigned: zeniko)

Details

(4 keywords, Whiteboard: [sg:high])

Attachments

(3 files, 1 obsolete file)

1.9 branch patch
2.14 KB, patch
dietrich
: review+
dveditz
: approval1.9.0.6+
Details | Diff | Splinter Review
1.8.1 branch patch
2.23 KB, patch
dveditz
: approval1.8.1.next+
Details | Diff | Splinter Review
patch and test
8.19 KB, patch
dietrich
: review+
beltzner
: approval1.9.1+
Details | Diff | Splinter Review 
It's possible to change the type of an input control during restoration.
Keywords: testcase
Whiteboard: [sg:high]
Attached patch patch and test (obsolete) — — Splinter Review 
In order to keep sessionstore.js small and to remain compatible with Betas 1 and 2, this patch special-cases <input type="file">. AFAICT this is the only privacy sensitive of our input elements (please correct me now, if I'm wrong!).

On a side-note: The same vulnerability works the other way round, as well: If the user selects a file, it'd be possible to get the full file path instead of just the filename. Both patch and test take care of both ways.
Assignee: nobody → zeniko
Status: NEW → ASSIGNED
Attachment #350380 - Flags: review?(dietrich)
Attached patch 1.9 branch patch — — Splinter Review 

    
    

    
  


  

    
    

    
  
(In reply to comment #2)
> AFAICT this is the only privacy sensitive of our input elements

... besides <input type="password"> which we single out already.

    
    

    
  
Do we still need 1.8.1 patches, anyway?
Flags: blocking1.9.0.6?
Flags: blocking1.8.1.next?
Flags: blocking-firefox3.1?

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch and test
        — 
          — Splinter Review
      

    


  
Minor update to the test: Let's also make sure that <input type="file"> restoration wasn't broken in the same circumstances.

        Attachment #350380 -
        Attachment is obsolete: true

        Attachment #350477 -
        Flags: review?(dietrich)

        Attachment #350380 -
        Flags: review?(dietrich)
Flags: blocking-firefox3.1? → blocking-firefox3.1+
Whiteboard: [sg:high] → [sg:high][has patch][needs review dietrich]

    
    

    
  
This has been placed on our "Top Security Bugs" list.  Please treat as a top priority.

    
  

        Attachment #350477 -
        Flags: review?(gavin.sharp)

    
  
Whiteboard: [sg:high][has patch][needs review dietrich] → [sg:high][has patch][needs review dietrich or gavin]

    
    

    
  
Comment on attachment 350477 [details] [diff] [review]
patch and test

looks fine, r=me

        Attachment #350477 -
        Flags: review?(dietrich) → review+

    
  
OS: Windows XP → All
Hardware: PC → All
Whiteboard: [sg:high][has patch][needs review dietrich or gavin] → [sg:high][has patch][needs approval]

    
  

        Attachment #350477 -
        Flags: review?(gavin.sharp) → approval1.9.1?

    
  

        Attachment #350382 -
        Flags: approval1.9.0.6?

    
  

        Attachment #350382 -
        Flags: approval1.9.0.6? → approval1.8.1.next?

    
  

        Attachment #350381 -
        Flags: approval1.9.0.6?

    
    

    
  
(In reply to comment #6)
> Do we still need 1.8.1 patches, anyway?

Several vendors will continue to support Firefox 2 longer than MoCo, so yeah, it's extremely helpful.
Flags: wanted1.9.0.x+
Flags: wanted1.8.1.x+
Flags: blocking1.9.0.6?
Flags: blocking1.9.0.6+

    
    

    
  
We're going to wait for 1.9.1 approval and landing before approving for the older branches, but why is 1.9.1 so different from 1.9.0 here? That would seem to call for a separate review.

    
    

    
  
Comment on attachment 350381 [details] [diff] [review]
1.9 branch patch

(In reply to comment #11)
> why is 1.9.1 so different from 1.9.0 here?

Because with all the improvements on 1.9.1 we can actually distinguish type="file" from type="text" and restore both whereas on older branches we can't and thus just have to ignore type="file" during both saving and restoring.

Dietrich: The second bit has been included in the 1.9.1 patch and the first bit is equal to the type="password" one. Please nod if I haven't missed anything.

        Attachment #350381 -
        Flags: review?(dietrich)

        Attachment #350381 -
        Flags: review?(dietrich) → review+

    
    

    
  
Comment on attachment 350477 [details] [diff] [review]
patch and test

a191=beltzner

        Attachment #350477 -
        Flags: approval1.9.1? → approval1.9.1+

    
  
Keywords: checkin-needed
Whiteboard: [sg:high][has patch][needs approval] → [sg:high][has patch][needs branch approval]
Whiteboard: [sg:high][has patch][needs branch approval] → [sg:high][has patch][needs trunk/1.9.1 landing]

    
    

    
  
http://hg.mozilla.org/mozilla-central/rev/8d0d5017c101
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Whiteboard: [sg:high][has patch][needs trunk/1.9.1 landing] → [sg:high][has patch][needs 1.9.1 landing]
Target Milestone: --- → Firefox 3.2a1
Whiteboard: [sg:high][has patch][needs 1.9.1 landing] → [sg:high][has patch]

    
    

    
  
Comment on attachment 350381 [details] [diff] [review]
1.9 branch patch

Approved for 1.9.0.6, a=dveditz for release-drivers.

        Attachment #350381 -
        Flags: approval1.9.0.6? → approval1.9.0.6+

    
    

    
  
dveditz, please approve blocking1.8.1.next if you come to it.
Flags: wanted1.8.0.x-
Flags: blocking1.8.0.next-
Flags: blocking1.8.1.next? → blocking1.8.1.next+

    
    

    
  
checked into 1.9.0 branch:

Checking in browser/components/sessionstore/src/nsSessionStore.js;
/cvsroot/mozilla/browser/components/sessionstore/src/nsSessionStore.js,v  <--  nsSessionStore.js
new revision: 1.108; previous revision: 1.107
done
Keywords: fixed1.9.0.6

    
    

    
  
Removing checkin-needed keyword, as I think this has been completely landed, but please correct me if I'm wrong.
Keywords: checkin-needed
Whiteboard: [sg:high][has patch] → [sg:high]

    
    

    
  
This hasn't landed on the 1.8.1 branch yet. Needs approval first, though.
Whiteboard: [sg:high] → [sg:high][approval needed for 1.8.1.next]

    
    

    
  
yep. we are waiting for approval1.8.1.next? from comment 16 ... drivers please support us!

    
    

    
  
Verified for 1.9.0.6 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.6pre) Gecko/2009010606 GranParadiso/3.0.6pre.
Keywords: fixed1.9.0.6 → verified1.9.0.6

    
    

    
  
Comment on attachment 350382 [details] [diff] [review]
1.8.1 branch patch

Approved for 1.8 branch, a=dveditz

        Attachment #350382 -
        Flags: approval1.8.1.next? → approval1.8.1.next+

    
  
Keywords: checkin-needed
Whiteboard: [sg:high][approval needed for 1.8.1.next] → [sg:high][checkin needed: 1.8.1 branch]

    
    

    
  
MOZILLA_1_8_BRANCH:

Checking in browser/components/sessionstore/src/nsSessionStore.js;
/cvsroot/mozilla/browser/components/sessionstore/src/nsSessionStore.js,v  <--  nsSessionStore.js
new revision: 1.5.2.55; previous revision: 1.5.2.54
done
Keywords: checkin-needed → fixed1.8.1.21
Whiteboard: [sg:high][checkin needed: 1.8.1 branch] → [sg:high]
Group: core-security
Alias: CVE-2009-0355

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: