Closed Bug 790146 Opened 7 years ago Closed 7 years ago

Fix some bogus OOM-handling in JSScript methods

Categories

(Core :: JavaScript Engine, defect)

16 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18
Tracking Status
firefox-esr10 --- unaffected
firefox-esr17 --- wontfix
b2g18 --- fixed

People

(Reporter: njn, Assigned: njn)

References

Details

(Keywords: sec-audit)

Attachments

(1 file)

While looking at bug 787887 I found some related problems with OOM-handling.
I don't think they explain the crash in bug 787887 (which is a 
NULL-dereference), but they're worth fixing anyway.  I'm marking this
security-sensitive just to be safe.

- In JSScript::ensureHasDebugScript() we would erroneously delete
  compartment()->debugScriptMap if the table insertion failed, despite the 
  fact that it is a valid HashTable pointer.  This would leave
  compartment()->debugScriptMap as a dangling pointer.

- The same problem occurred in JSScript::initScriptCounts().  
  
- In initScriptCounts() we were also erroneously deleting |cursor| instead of
  |base| in that same OOM path.  I.e. it's a bogus delete that would 
  probably cause a crash if it was hit.
  
You'll need to look at the original code to understand the patches;  eight 
lines isn't enough context.
While looking at bug 787887 I found some related problems with OOM-handling.
I don't think they explain the crash in bug 787887 (which is a 
NULL dereference), but they're worth fixing anyway.  Marking
security-sensitive just to be safe.

- In JSScript::ensureHasDebugScript() we would erroneously delete
  compartment()->debugScriptMap if the table insertion failed, despite the 
  fact that it is a valid HashTable pointer.  This would leave
  compartment()->debugScriptMap as a dangling pointer.

- The same problem occurred in JSScript::initScriptCounts().  
  
- In initScriptCounts() we were also erroneously deleting |cursor| instead of
  |base| in that same OOM path.  I.e. this passes |delete| a bogus pointer,
  which could easily cause a crash if executed.
  
You'll need to look at the original code to understand the patches;  eight 
lines isn't enough context.
Attachment #659978 - Flags: review?(jorendorff)
Keywords: sec-audit
Comment on attachment 659978 [details] [diff] [review]
Slightly improve some JSScript OOM handling.

jorendorff:  review ping.  The patch is small and this is an s-s bug.
Attachment #659978 - Flags: review?(jorendorff) → review+
https://hg.mozilla.org/mozilla-central/rev/1c7689d7feb6
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
Group: core-security
You need to log in before you can comment on or make changes to this bug.