Closed Bug 1268056 Opened 8 years ago Closed 8 years ago

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

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla49
Tracking Status
firefox47 --- unaffected
firefox48 + verified
firefox49 + verified
firefox-esr45 --- unaffected

People

(Reporter: decoder, Assigned: arai)

References

Details

(5 keywords, Whiteboard: [fuzzblocker] [jsbugmon:update])

Crash Data

Attachments

(1 file)

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
This happens fairly often, marking as fuzzblocker.
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update,bisect][fuzzblocker]
Flags: needinfo?(arai.unmht)
Group: javascript-core-security
Forgot that rx can be non-RegExp object.
This also affects mozilla48 (now aurora), but it will be backed out in bug 1265307, right?
Assignee: nobody → arai.unmht
Flags: needinfo?(arai.unmht)
Attachment #8746305 - Flags: review?(hv1989)
Attachment #8746305 - Flags: review?(hv1989) → review+
Will be backed out in FF48 indeed by bug 1265307. Letting it depend on it. So I can find it back. Just in case.
Blocks: 1265307
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.
Attachment #8746305 - Flags: sec-approval?
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
Flags: needinfo?(dveditz)
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.
Flags: needinfo?(dveditz)
Keywords: sec-high
Attachment #8746305 - Flags: sec-approval? → sec-approval+
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
Whiteboard: [jsbugmon:update,bisect][fuzzblocker] → [fuzzblocker] [jsbugmon:update]
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.
https://hg.mozilla.org/mozilla-central/rev/1d4795f0b4e2
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Status: RESOLVED → VERIFIED
JSBugMon: This bug has been automatically verified fixed.
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
Attachment #8746305 - Flags: approval-mozilla-aurora?
Group: javascript-core-security → core-security-release
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
Attachment #8746305 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
JSBugMon: This bug has been automatically verified fixed on Fx48
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.