Crash [@ ??] or Assertion failure: Length should be greater than 0., at js/src/jit/MacroAssembler.cpp:1454

VERIFIED FIXED in Firefox 48

Status

()

defect
--
critical
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: decoder, Assigned: arai)

Tracking

(Blocks 1 bug, 4 keywords)

Trunk
mozilla48
x86
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 unaffected, firefox48 verified)

Details

(Whiteboard: [fuzzblocker] [jsbugmon:update], crash signature)

Attachments

(1 attachment)

Reporter

Description

3 years ago
The following testcase crashes on mozilla-central revision d62963756d9a (build with --enable-optimize --enable-posix-nspr-emulation --enable-valgrind --enable-gczeal --target=i686-pc-linux-gnu --disable-tests --disable-debug, run with --fuzzing-safe --ion-eager --ion-offthread-compile=off):

 function P() myRegExp = {
     get constructor() {
         return {
             get [Symbol.species]() {
                 return function() {
                     return {
                         get lastIndex() {
                             return lastIndexResult[n]
                         },
                         set lastIndex(v) {},
                         get exec() {
                             return function() {
                                 return [n++]
                             }
                         }
                     }
                 }
             }
         }
     }
 }
 n = 0
 target = "abcde"
 execResult = [P()]
 lastIndexResult = [, 9]
 RegExp.prototype[Symbol.split].call(myRegExp, target)


Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0xf7c7cf71 in ?? ()
#0  0xf7c7cf71 in ?? ()
eax	0x0	0
ebx	0xf6000000	-167772160
ecx	0xf5ffca59	-167785895
edx	0xf5000420	-184548320
esi	0xf4f65060	-185184160
edi	0xfef65064	-17411996
ebp	0x9	9
esp	0xffffc88c	4294953100
eip	0xf7c7cf71	4157067121
=> 0xf7c7cf71:	mov    %al,(%ebx)
   0xf7c7cf73:	add    $0x1,%ecx


Marking s-s due to crash on heap with bad address.
Reporter

Comment 1

3 years ago
Needinfo on :arai, this is a fuzzblocker.
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update,bisect][fuzzblocker]
Assignee

Comment 2

3 years ago
This is a regression from bug 887016.
Will post a patch shortly.
Blocks: 887016
Assignee

Comment 3

3 years ago
the value of lastIndex is assigned to e, and it's assigned to p, and then assigned to q, and q is compared with size.
so, when |q < size| gets false, the value of p could be |p > size|.
we should check that case and put empty string to A.
Assignee: nobody → arai.unmht
Attachment #8740341 - Flags: review?(till)
Comment on attachment 8740341 [details] [diff] [review]
Check lastIndex after loop in RegExp.prototype[@@split].

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

r=me
Attachment #8740341 - Flags: review?(till) → review+
Can someone suggest a security rating for this issue?
Assignee

Comment 6

3 years ago
It should be sec-high (Obtain confidential data from other sites the user is visiting or the local machine).

With this bug, the function can return a JS string that's length is negative,
it means that the data on the heap can be read through the string.


This is a recent-nightly-only regression from bug 887016, so I believe the sec-approval is not needed.
I'll land this patch shortly.
Assignee

Comment 7

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/1fd59cb6c8686e0874fc99431e1032144605994c
Bug 1263851 - Check lastIndex after loop in RegExp.prototype[@@split]. r=till
https://hg.mozilla.org/mozilla-central/rev/1fd59cb6c868
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48

Updated

3 years ago
Whiteboard: [jsbugmon:update,bisect][fuzzblocker] → [fuzzblocker] [jsbugmon:update]
Group: javascript-core-security → core-security-release

Updated

3 years ago
Status: RESOLVED → VERIFIED

Comment 9

3 years ago
JSBugMon: This bug has been automatically verified fixed.
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.