Closed Bug 288688 Opened 16 years ago Closed 16 years ago

JS "lambda" replace exposes malloc heap space after end of JS string

Categories

(Core :: JavaScript Engine, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla1.8beta2

People

(Reporter: brendan, Assigned: brendan)

References

()

Details

(Keywords: fixed-aviary1.0.3, fixed1.7.7, js1.5, Whiteboard: [sg:fix])

Attachments

(2 files, 3 obsolete files)

Quick and dirty patch next.  There's a better patch that should be done, but the
patch I'll attach next fixes this, AFAICT.

/be
Attached patch quick and dirty fix (obsolete) — Splinter Review
Really, all of cx->regExpStatics should be stacked and unstacked around the
lambda invocations.

/be
Attachment #179311 - Flags: review?(shaver)
Severity: normal → critical
Status: NEW → ASSIGNED
Flags: blocking1.7.7?
Flags: blocking-aviary1.0.3?
Priority: -- → P1
Summary: "lambda" replace exposes malloc heap space after end of JS string → JS "lambda" replace exposes malloc heap space after end of JS string
Target Milestone: --- → mozilla1.8beta2
Attached patch better fix (obsolete) — Splinter Review
Lacking block scope and dynamically scoped variables, JS has never tried to
emulate Perl's dynamic scope for $1, $&, etc. with the pre-ECMA extensions
analogous to those vars (RegExp.$1, RegExp.lastMatch, etc).  Here's a case
where it must.

/be
Attachment #179313 - Flags: review?(shaver)
Group: security
*** Bug 288608 has been marked as a duplicate of this bug. ***
*** Bug 288685 has been marked as a duplicate of this bug. ***
Flags: blocking-aviary1.1+
Whiteboard: [sg:fix]
Attached file testcase
preserving testcase for the future in case the cubic.xfo.org.ru blog ever goes
away.
Attachment #179313 - Flags: superreview?(dbaron)
BTW, this bug is like 8+ years old.  Roger Lawrence fixed half of it in 2000:

r=norris,waldemar
Fixes for bugs#23607, 23608, 23610, 23612, 23613. Also, first cut at URI
encode & decode routines.

Unfortunately, AFAICT none of the bugs he cites had anything to do with the two
hunks of that revision:

@@ -1061,16 +1080,22 @@ find_replen(JSContext *cx, ReplaceData *
@@ -1138,16 +1163,17 @@ find_replen(JSContext *cx, ReplaceData *

that half-fixed the original 1997-era bug.

/be
Attachment #179311 - Flags: review?(shaver)
Comment on attachment 179313 [details] [diff] [review]
better fix

dbaron shoots, he scores: cx->regExpStatics.moreParens points to malloc'ed
space that could be realloc'ed by the inner match.

/be
Attachment #179313 - Attachment is obsolete: true
Attachment #179313 - Flags: superreview?(dbaron)
Attachment #179313 - Flags: superreview-
Attachment #179313 - Flags: review?(shaver)
Attached patch fix, v3 (obsolete) — Splinter Review
Attachment #179311 - Attachment is obsolete: true
Attachment #179344 - Flags: superreview?(dbaron)
Attachment #179344 - Flags: review?(shaver)
Comment on attachment 179344 [details] [diff] [review]
fix, v3

I am a crispy critter right now.  This leaks the inner moreParens, if
allocated.  dbaron is two for two.

/be
Attachment #179344 - Attachment is obsolete: true
Attachment #179344 - Flags: superreview?(dbaron)
Attachment #179344 - Flags: review?(shaver)
Attached patch fix, v4Splinter Review
Attachment #179347 - Flags: superreview?(dbaron)
Attachment #179347 - Flags: review?(shaver)
Attachment #179347 - Flags: superreview?(dbaron) → superreview+
Fixed on trunk, AVIARY_1_0_1_20050124_BRANCH, and MOZILLA_1_7_BRANCH.

Thanks for the report, I hope that's the last bug from 1997 left ;-).

/be
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: blocking1.7.7?
Flags: blocking1.7.7+
Flags: blocking-aviary1.0.3?
Flags: blocking-aviary1.0.3+
Resolution: --- → FIXED
js/tests/js1_5/Regress/regress-288688.js checked in.
Depends on: 288818
This is assigned to SA14821 (including Suite 1.7.6)
http://secunia.com/advisories/14821/

and SA14820 (Firefox 1.0.1 and 1.0.2)
http://secunia.com/advisories/14820/

Link to "Mozilla Products Arbitrary Memory Exposure Test" page:
http://secunia.com/mozilla_products_arbitrary_memory_exposure_test/


*** Bug 289134 has been marked as a duplicate of this bug. ***
Blocks: sbb?
*** Bug 289194 has been marked as a duplicate of this bug. ***
*** Bug 290161 has been marked as a duplicate of this bug. ***
*** Bug 290310 has been marked as a duplicate of this bug. ***
Flags: testcase+
This bug is not entirely solved!!

If you go to the secunia site
http://secunia.com/mozilla_products_arbitrary_memory_exposure_test

and do the test with 1.0.2, it shows some contents of the memory.

Now with 1.0.3 (linux) it only shows some X's, but the entire system crashes
after doing this test a few times!!

Dont know if this is another problem, but here it is, since it's triggered by
the same thing..
This bug was fixed after Firefox 1.0.2. I just checked secunia with Firefox
1.0.4 and it does not show the 'XXX'. Please upgrade to 1.0.4 as it contains
significant security updates and retest.
(In reply to comment #19)
> 1.0.4 and it does not show the 'XXX'. 

Heh! It SHOULD show "XXX..." because it's correct behaviour of the script.
(In reply to comment #20)

> Heh! It SHOULD show "XXX..." because it's correct behaviour of the script.

You are right of course, doh! but it looks like a variant of bug 293940 prevents
the string from displaying. You can check that the mem string is 'XXX...X' by
setting a breakpoint at line 247 in the page where 
document.getElementById('result').value = mem;

Blocks: sbb+
No longer blocks: sbb?
verified fixed.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.