776484 - Assertion failure: end >= begin, at ./dist/include/mozilla/Util.h:297 with decompileBody

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following test asserts on mozilla-central revision 462106f027af (no options required):

decompileBody(function () { });


This might be debug-only (it only seems to work with decompileBody, not with toString), but until that's confirmed, I'll mark it s-s since it's likely some offset/range problem that could be exploitable.

Also note that this test is whitespace sensitive (you need at least one space in the function body).

Comment 1

[:Benjamin Peterson]

Attachment: only strip whitespace on expression bodies

I think not being smart about removing whitespace about bodies is braces is a better behavior anyway.

Comment 2

[Jason Orendorff [:jorendorff]]

Comment on attachment 645003 [details] [diff] [review]
only strip whitespace on expression bodies

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

::: js/src/jsfun.cpp
@@ +664,5 @@
>                      // comment.
>                      if (!out.append("/* use strict */ "))
>                          return NULL;
>                  } else {
> +                    if (!out.append("\n\"use strict\";\n"))

This is unrelated to the crash, right?

I don't think it's necessary, but if you think the output looks nicer this way, ok.

Comment 3

[David Bolter [:davidb] (NeedInfo me for attention)]

Sounds like sec-critical.

Comment 4

[:Benjamin Peterson]

(In reply to Jason Orendorff [:jorendorff] from comment #2)
> Comment on attachment 645003 [details] [diff] [review]
> only strip whitespace on expression bodies
> 
> Review of attachment 645003 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/jsfun.cpp
> @@ +664,5 @@
> >                      // comment.
> >                      if (!out.append("/* use strict */ "))
> >                          return NULL;
> >                  } else {
> > +                    if (!out.append("\n\"use strict\";\n"))
> 
> This is unrelated to the crash, right?


Correct. But it's related to the fix. :)

> 
> I don't think it's necessary, but if you think the output looks nicer this
> way, ok.

I did it because it at least guarantees "use strict"; is on its own line.

Comment 5

[:Benjamin Peterson]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ffda09fa2ca3

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/ffda09fa2ca3

Comment 7

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 8

[Daniel Veditz [:dveditz]]

I'm interpreting that this "blocks" bug 761723 to mean it's a regression from it, and therefore earlier branches do not need this fix. If they do please request approval to land the patch on those branches and clear the "unaffected" statuses.

Comment 9

[u279076]

(In reply to Christian Holler (:decoder) from comment #7)
> JSBugMon: This bug has been automatically verified fixed.

Looks like the automated verifier does not know how to set status flags, doing so now.

Comment 10

[Christian Holler (:decoder)]

Automatically extracted testcase for this bug was committed:

https://hg.mozilla.org/mozilla-central/rev/efaf8960a929