Closed Bug 655098 Opened 13 years ago Closed 13 years ago

CreateNPObjectMember jsids don't appear to be rooted

Categories

(Core Graveyard :: Plug-ins, defect)

Product:
Core Graveyard
Component:
Plug-ins
Platform:
x86
Windows 7
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox5- wontfix, firefox6+ fixed, firefox7+ fixed, blocking1.9.2 .23+, status1.9.2 .23-fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla6
Tracking Flags:
Tracking Status
firefox5 - wontfix
firefox6 + fixed
firefox7 + fixed
blocking1.9.2 --- .23+
status1.9.2 --- .23-fixed

People

(Reporter: benjamin, Assigned: benjamin)

Details

(Whiteboard: [sg:critical?][qa-ntd-192][qa-])

Attachments

(2 files)

Root the NPObjectMember id, rev. 1
1.20 KB, patch
cdleary
: review+
mrbkap
: review+
christian
: approval-mozilla-aurora+
Details | Diff | Splinter Review
1.9.2 branch version, maybe, rev. 1
1.20 KB, patch
cdleary
: review+
christian
: approval1.9.2.20+
Details | Diff | Splinter Review 
The jsid in CreateNPObjectMember may be a non-rooted identifier. http://mxr.mozilla.org/mozilla-central/source/dom/plugins/base/nsJSNPRuntime.cpp#2089

It should be traced at http://mxr.mozilla.org/mozilla-central/source/dom/plugins/base/nsJSNPRuntime.cpp#2292

But currently the JSAPI doesn't actually expose a public function for tracing jsid.

Filing as private becuase it's theoretically possible to abuse this via script to perhaps dereference string characters which were GCed.
(In reply to comment #0)
> But currently the JSAPI doesn't actually expose a public function for tracing
> jsid.

We should add one, but in the meantime you can do if (JSID_IS_STRING(id)) JS_CALL_STRING_TRACER(trc, JSID_TO_STRING(id), "some_name")
Assignee: nobody → benjamin
Status: NEW → ASSIGNED

        Attachment #530636 -
        Flags: review?(mrbkap)

        Attachment #530636 -
        Flags: review?(cdleary)

        Attachment #530636 -
        Flags: review?(mrbkap) → review+

    
    

    
  
Comment on attachment 530636 [details] [diff] [review]
Root the NPObjectMember id, rev. 1

Review of attachment 530636 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry I didn't see this earlier. Got buried in my email.

        Attachment #530636 -
        Flags: review?(cdleary) → review+

    
    

    
  
summary sounds bad, guessing at severity rating. Is this in old code (affecting 1.9.2.x as well)?

          status1.9.2:
          --- → ?

          status-firefox5:
          --- → affected

          status-firefox6:
          --- → affected

          status-firefox7:
          --- → affected
Whiteboard: [sg:critical?]

    
    

    
  
Yes, this affects all older branches. I guess it's possible to exploit this... not easy, though.

    
    

    
  
Can we get this patch landed?

          status-firefox5:
          affected → wontfix

          tracking-firefox5:
          --- → -

          tracking-firefox6:
          --- → +

          tracking-firefox7:
          --- → +
Keywords: checkin-needed

    
    

    
  
http://hg.mozilla.org/mozilla-central/rev/49e57fa259dc
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED

    
  
Target Milestone: --- → mozilla7

        Attachment #530636 -
        Flags: approval-mozilla-aurora?

    
    

    
  
Comment on attachment 530636 [details] [diff] [review]
Root the NPObjectMember id, rev. 1

Approved for mozilla-aurora

        Attachment #530636 -
        Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

    
  
Keywords: checkin-needed
blocking1.9.2: --- → ?

          status1.9.2:
          ? → wanted

          status-firefox7:
          affected → fixed

    
  
blocking1.9.2: ? → .19+

    
    

    
  
bsmedberg, can we get this landed on aurora?

    
    

    
  
Do we need a different patch for the 1.9.2 branch?

    
    

    
  
Comment on attachment 530636 [details] [diff] [review]
Root the NPObjectMember id, rev. 1

This patch will probably apply to 1.9.2 manually using the old location (modules/plugin/base/src) instead of dom/plugins/base.

        Attachment #530636 -
        Flags: checkin?

    
  

        Attachment #530636 -
        Flags: checkin?

    
    

    
  


  
Somehow this got marked a branch blocker without me noticing. This might be the correct branch version of this, but I'm not sure and I'm not sure it's really serious enough to block.

        Attachment #549927 -
        Flags: review?(cdleary)

        Attachment #549927 -
        Flags: review?(cdleary) → review+

    
    

    
  
Comment on attachment 549927 [details] [diff] [review]
1.9.2 branch version, maybe, rev. 1

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/48b6fb82a960

        Attachment #549927 -
        Flags: approval1.9.2.20+

    
    

    
  
Are there any STR for this issue to use for verification of this fix?
blocking1.9.2: .20+ → .21+

          status1.9.2:
          .20-fixed → .21-fixed
Whiteboard: [sg:critical?] → [sg:critical?][qa-examined-192][qa-needs-STR]

    
    

    
  
No, it was entirely theoretical.

    
    

    
  
Ok. Marking this as "Nothing to Do" for QA.
Whiteboard: [sg:critical?][qa-examined-192][qa-needs-STR] → [sg:critical?][qa-ntd-192]

    
    

    
  
qa-: fix needs no QA verification
Whiteboard: [sg:critical?][qa-ntd-192] → [sg:critical?][qa-ntd-192][qa-]
Group: core-security

    
  
Product: Core → Core Graveyard

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: