Last Comment Bug 339785 - scanner: memory exposure to scripts
: scanner: memory exposure to scripts
Status: VERIFIED FIXED
[sg:critical?]
: verified1.8.0.5, verified1.8.1
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Igor Bukanov
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-05-31 03:11 PDT by Igor Bukanov
Modified: 2007-02-08 16:13 PST (History)
3 users (show)
dveditz: blocking1.8.0.5+
bob: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Test case for JS shell (529 bytes, text/plain)
2006-05-31 03:28 PDT, Igor Bukanov
no flags Details
Better test (for JS shell) (453 bytes, text/plain)
2006-05-31 03:46 PDT, Igor Bukanov
no flags Details
Fix (814 bytes, patch)
2006-05-31 04:05 PDT, Igor Bukanov
mrbkap: review+
brendan: approval‑branch‑1.8.1+
dveditz: approval1.8.0.5+
Details | Diff | Review
Even better test case (545 bytes, text/plain)
2006-05-31 12:14 PDT, Igor Bukanov
no flags Details
e4x/GC/regress-339785.js (2.92 KB, text/plain)
2006-06-16 08:40 PDT, Bob Clary [:bc:]
no flags Details

Description Igor Bukanov 2006-05-31 03:11:07 PDT
The code of GetXMLEntity from jsscan.c contains the following code:

    bp = ts->tokenbuf.base + offset;
....
    FastAppendChar(&ts->tokenbuf, ';');
    bytes = js_DeflateString(cx, bp + 1,
                             PTRDIFF(ts->tokenbuf.ptr, bp, jschar) - 2);
 
This does not take into account that FastAppendChar may grow and reallocate the buffer. When such reallocation happens, ts->tokenbuf.ptr - bp can be an arbitrary number. If js_DeflateString can allocate and access the corresponding region of memory, bytes would be C string corresponding to this arbitrary number of characters. Then this string would be embedded in the throw error object that the script can catch and read.

Since in principle this could allow to read various bytes that travels through arena area via catching from script the thrown error object, I mark this as security problem.
Comment 1 Igor Bukanov 2006-05-31 03:28:35 PDT
Created attachment 223899 [details]
Test case for JS shell

Note that depending on memory layout the test case may actually. If this is the case, try to change N in test() function to 127, 511 or other 2^p - 1 values.
Comment 2 Igor Bukanov 2006-05-31 03:46:07 PDT
Created attachment 223901 [details]
Better test (for JS shell)

This is a better version as in the previous case that "must_be_good" string could be bad in fact.
Comment 3 Igor Bukanov 2006-05-31 04:05:53 PDT
Created attachment 223907 [details] [diff] [review]
Fix

The fix just remove appending of ';' to the buffer as it was not included in the error message in any case.
Comment 4 Igor Bukanov 2006-05-31 12:12:58 PDT
I committed the fix
Comment 5 Igor Bukanov 2006-05-31 12:14:26 PDT
Created attachment 223958 [details]
Even better test case

The test case now contains a loop to try various N itself.
Comment 6 Brendan Eich [:brendan] 2006-05-31 14:07:40 PDT
Comment on attachment 223907 [details] [diff] [review]
Fix

Wow, what was I thinking?  Thanks,

/be
Comment 7 Igor Bukanov 2006-05-31 16:07:46 PDT
I committed the patch to MOZILLA_1_8_BRANCH
Comment 8 Daniel Veditz [:dveditz] 2006-06-13 14:59:55 PDT
Comment on attachment 223907 [details] [diff] [review]
Fix

approved for 1.8.0 branch, a=dveditz for drivers
Comment 9 Igor Bukanov 2006-06-13 15:14:04 PDT
I committed the patch from comment 3 to MOZILLA_1_8_0_BRANCH
Comment 10 Bob Clary [:bc:] 2006-06-16 08:40:53 PDT
Created attachment 225875 [details]
e4x/GC/regress-339785.js
Comment 11 Bob Clary [:bc:] 2006-06-28 13:17:43 PDT
verified fixed 1.8.0.5, 1.8.1, 1.9a1 20060622 builds on all platforms.
Comment 12 Bob Clary [:bc:] 2006-07-25 00:51:28 PDT
note to self: e4x/GC/regress-339785.js: TIMED OUT browser firefox-trunk-dbg-1.9a1_2006072314 windows, not seen on any other platform.
Comment 13 Alexander Sack 2006-07-28 07:03:15 PDT
'Even better testcase' appeaers to be okay aviary. So I guess this is not present there. Igor?
Comment 14 Igor Bukanov 2006-07-28 07:31:08 PDT
(In reply to comment #13)
> 'Even better testcase' appeaers to be okay aviary. So I guess this is not
> present there. Igor?
> 

The bug was introduces during 1.8 development. So assuming "aviary" means branches with version < 1.8, then the answer is that the bug does not exist there.
Comment 15 Bob Clary [:bc:] 2007-02-08 16:13:59 PST
/cvsroot/mozilla/js/tests/e4x/GC/regress-339785.js,v  <--  regress-339785.js

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