Closed Bug 1350464 Opened 7 years ago Closed 7 years ago

Crash [@ js::frontend::TokenStream::getTokenInternal]

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr45 --- unaffected
firefox52 --- unaffected
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 --- fixed

People

(Reporter: gkw, Assigned: Waldo)

References

Details

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

Crash Data

Attachments

(3 files)

The following testcase crashes on mozilla-central revision 4c987b7ed54a (build with --enable-debug, run with --fuzzing-safe --no-threads --no-baseline --no-ion):

oomTest(function() {
    try {



























































































































    } catch(e) {
        ;
    }
})

Backtrace:

#0  0x0000000000d3ec28 in js::frontend::TokenStream::getTokenInternal (this=0x7fffa1836558, ttp=0x7fffa18359ac, modifier=modifier@entry=js::frontend::Token::Operand) at js/src/frontend/TokenStream.cpp:1942
#1  0x00000000004a0f0a in js::frontend::TokenStream::peekToken (this=this@entry=0x7fffa1836558, ttp=ttp@entry=0x7fffa18359ac, modifier=js::frontend::Token::Operand, modifier=js::frontend::Token::Operand) at js/src/frontend/TokenStream.h:603
#2  0x00000000004d58c3 in js::frontend::Parser<js::frontend::FullParseHandler>::statementList (this=this@entry=0x7fffa1836540, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName) at js/src/frontend/Parser.cpp:4029
#3  0x00000000004d60f1 in js::frontend::Parser<js::frontend::FullParseHandler>::catchBlockStatement (this=this@entry=0x7fffa1836540, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, catchParamScope=...) at js/src/frontend/Parser.cpp:6918
#4  0x00000000004d679c in js::frontend::Parser<js::frontend::FullParseHandler>::tryStatement (this=this@entry=0x7fffa1836540, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName) at js/src/frontend/Parser.cpp:6841
#5  0x00000000004d545b in js::frontend::Parser<js::frontend::FullParseHandler>::statementListItem (this=this@entry=0x7fffa1836540, yieldHandling=yieldHandling@entry=js::frontend::YieldIsName, canHaveDirectives=<optimized out>) at js/src/frontend/Parser.cpp:7580
/snip

For detailed crash information, see attachment.
Attached file testcase
I have attached the testcase again in case the whitespaces (which are required) do not show up properly.
Assignee: nobody → gary
Whoops, still getting used to the new b.m.o interface. It might be related to bug 1326454 as the possible regressor, so pushing over to :Waldo to take a look.
Assignee: gary → jwalden+bmo
Flags: needinfo?(jwalden+bmo)
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/65f852c28e71
user:        Jeff Walden
date:        Wed Jan 04 14:45:37 2017 -0800
summary:     Bug 1326454 - Make TokenStream::updateLineInfoForEOL fallible, and immediately handle errors when it fails instead of temporarily deferring such handling.  r=arai
Blocks: 1326454
Comment on attachment 8851193 [details] [diff] [review]
Don't try to poison not-yet-allocated structures when an error occurs during parsing

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

::: js/src/frontend/TokenStream.cpp
@@ +1939,5 @@
>  
>    error:
> +    // We didn't get a token, so don't set |flags.isDirtyLine|.  And don't
> +    // poison any of |*tp|: if we haven't allocated a token, |tp| could be
> +    // uninitialized.

error is reachable both from before and after |tp = newToken(-1);|,
it looks like we should set |tp = nullptr| at the beginning and poison it when tp is non-null.
Comment on attachment 8851193 [details] [diff] [review]
Don't try to poison not-yet-allocated structures when an error occurs during parsing

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

discussed in IRC, and it should be okay not to poison it there
Attachment #8851193 - Flags: review?(arai.unmht) → review+
Marking as fuzzblocker due to the amount of crashes I see with this signature and s-s because several of them have a non-zero crash address.
Group: javascript-core-security
Whiteboard: [jsbugmon:update] → [jsbugmon:update][fuzzblocker]
https://hg.mozilla.org/mozilla-central/rev/06e1e7de67bc
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Woops, didn't realize this was (properly) marked sensitive.  Updating with the proper rating -- and I guess per my usual practice I should have landed the fix, then a week or so later landed the test, as a nicety to nightly users -- but this was only ever in nightly, so no forgotten intent to be careful about landing the fix for this.
Flags: needinfo?(jwalden+bmo)
Keywords: sec-critical
Group: javascript-core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: