1268056 - Crash [@ JSVAL_TO_STRING_IMPL] or Assertion failure: this->is<T>(), at js/src/jsobj.h:548

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase crashes on mozilla-central revision 52072b6bec14 (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --disable-tests --disable-debug, run with --fuzzing-safe --ion-offthread-compile=off):

RegExp.prototype[Symbol.split].call({})


Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000a531a0 in JSVAL_TO_STRING_IMPL (l=<error reading variable: Cannot access memory at address 0x8>) at js/src/opt64/dist/include/js/Value.h:778
#0  0x0000000000a531a0 in JSVAL_TO_STRING_IMPL (l=<error reading variable: Cannot access memory at address 0x8>) at js/src/opt64/dist/include/js/Value.h:778
#1  toString (this=0x8) at js/src/opt64/dist/include/js/Value.h:1272
#2  getSource (this=0x7ffff7e6e140) at js/src/vm/RegExpObject.h:451
#3  js::regexp_construct_no_sticky (cx=0x7ffff6907800, argc=<optimized out>, vp=0x7ffff31e81c8) at js/src/builtin/RegExp.cpp:535
#4  0x0000000000879fd1 in CallJSNative (args=..., native=0xa53130 <js::regexp_construct_no_sticky(JSContext*, unsigned int, JS::Value*)>, cx=0x7ffff6907800) at js/src/jscntxtinlines.h:235
[...]
#28 main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at js/src/shell/js.cpp:7471
rax	0x8	8
rbx	0x7ffff6907800	140737330051072
rcx	0x0	0
rdx	0x7fffffffc430	140737488340016
rsi	0x7fffffffffff	140737488355327
rdi	0x7fffffffffff	140737488355327
rbp	0x7ffff31e81c8	140737272250824
rsp	0x7fffffffbb80	140737488337792
r8	0x7fffffffffff	140737488355327
r9	0xfffbffffffffffff	-1125899906842625
r10	0x7ffff31e81c8	140737272250824
r11	0x7ffff31e1c00	140737272224768
r12	0x7fffffffbbb0	140737488337840
r13	0x7ffff6907830	140737330051120
r14	0xa53130	10826032
r15	0x2	2
rip	0xa531a0 <js::regexp_construct_no_sticky(JSContext*, unsigned int, JS::Value*)+112>
=> 0xa531a0 <js::regexp_construct_no_sticky(JSContext*, unsigned int, JS::Value*)+112>:	and    (%rax),%rdi
   0xa531a3 <js::regexp_construct_no_sticky(JSContext*, unsigned int, JS::Value*)+115>:	lea    0x50(%rbx),%rax

Comment 1

[Christian Holler (:decoder)]

This happens fairly often, marking as fuzzblocker.

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Check if |this| value is a RegExp object in the optimized path in RegExpSplit.

Forgot that rx can be non-RegExp object.
This also affects mozilla48 (now aurora), but it will be backed out in bug 1265307, right?

Comment 3

[Hannes Verschore [:h4writer]]

Will be backed out in FF48 indeed by bug 1265307. Letting it depend on it. So I can find it back. Just in case.

Comment 4

[Tooru Fujisawa [:arai]]

Comment on attachment 8746305 [details] [diff] [review]
Check if |this| value is a RegExp object in the optimized path in RegExpSplit.

[Security approval request comment]
> How easily could an exploit be constructed based on the patch?
I'm not sure how to exploit this, but this is a random address dereference, and some experienced people could exploit this.

> Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
It describes that there was a missing type check, so that it may cause reading wrong address, that may be controllable.

> Which older supported branches are affected by this flaw?
from mozilla-aurora (mozilla48)

> If not all supported branches, which bug introduced the flaw?
bug 1263340 (Part 3)

> Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
Same patch should be applicable (or maybe minor fix)

> How likely is this patch to cause regressions; how much testing does it need?
Less likely, this patch disables wrong optimized path and fallbacks to slow path, that was already working before the optimization.

Comment 5

[Al Billings [:abillings - ex-MoCo]]

This bug needs a security rating and only sec-high or sec-critical require security approvals. I assume that this is a sec-low if it really is just a non-exploitable. dereference.

https://wiki.mozilla.org/Security/Bug_Approval_Process

Comment 6

[Al Billings [:abillings - ex-MoCo]]

If it is really just aurora and trunk, we'll probably approve this for check-in no matter what but it would be good to understand the security implications here.

Comment 7

[Tooru Fujisawa [:arai]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1d4795f0b4e28db3ddf6a5751653c179fef68726
Bug 1268056 - Check if |this| value is a RegExp object in the optimized path in RegExpSplit. r=h4writer

Comment 8

[Fuzzing Team]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/86d33031bbd3
user:        Tooru Fujisawa
date:        Sat Apr 23 03:09:37 2016 +0900
summary:     Bug 1263340 - Part 3: Use internal slot for sticky flag in RegExp native functions. r=h4writer

This iteration took 185.607 seconds to run.

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/1d4795f0b4e2

Comment 10

[Fuzzing Team]

JSBugMon: This bug has been automatically verified fixed.

Comment 11

[Tooru Fujisawa [:arai]]

Comment on attachment 8746305 [details] [diff] [review]
Check if |this| value is a RegExp object in the optimized path in RegExpSplit.

same patch is applicable to mozilla-aurora

Approval Request Comment
> [Feature/regressing bug #]
bug 1263340

> [User impact if declined]
Possible exploit with random address access, treating partially user-controllable value as a String object,
or just crash.

> [Describe test coverage new/current, TreeHerder]
Tested in mozilla-central automated test.

> [Risks and why]
Low.  Added one more check for optimized path, and the certan case fallbacks to slow path.

> [String/UUID change made/needed]
None

Comment 12

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8746305 [details] [diff] [review]
Check if |this| value is a RegExp object in the optimized path in RegExpSplit.

OK to uplift to aurora, sec-high, potential crash fix

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/e11c1544850e

Comment 14

[Fuzzing Team]

JSBugMon: This bug has been automatically verified fixed on Fx48