[v8] Segmentation fault from preparser
Reported by
m...@mikepennisi.com,
Apr 15 2016
|
|||||||
Issue descriptionVersion: a8b02cbce1cc3e9e73ea24309ebd0008a29885da (Tue Apr 12 11:52:00 2016 -0700) OS: Ubuntu Linux Architecture: x64 When a function expression appears as the first element of a grouping operator, and it contains a function expression within a non-simple parameter, the preparser will encounter a segmentation fault and crash the process. $ d8 -e --min_preparse_length 1 '(function(_ = function() {}){})' Segmentation fault (core dumped) This is observable from programs loaded via a `<script>` tag in Chrome today, resulting in a crashed browser tab. I have attached a file that demonstrates this--it requires sufficient padding to trigger the preparsing optimization. As far as I can tell, these are the requirements: - The expression context must be created with a grouping operator - The function expression must be the first element of the grouping operator - The "outer" function object must be created from a FunctionExpression or a GeneratorExpression (not an ArrowFunction or ClassExpression) - The "inner" function object must not be an ArrowFunction Meaning that none of these forms trigger a segmentation fault: +function(_ = function(_ = function() {}) {} ('preceeding element', function(_ = function() {}) {}) ((_ = function() {}) => {}) (function(_ = () => {}) {}) The following aspects may vary: - The "inner" function may occur within any formal parameter - Either of the requisite functions may be "named" (i.e. specify a BindingIdentifier before the ParameterList) - The "inner" function can be any production that includes a FunctionBody - The "inner" function may appear as a default value or within a DestructuringBindingPattern - The code may be strict mode or non-strict mode Meaning that any of these forms *do* trigger a segmentation fault: (function(preceedingParameter, _ = function() {}) {}) (function x(_ = function y() {}) {}) (function(_ = function() {}) {}, 'additional element') (function* (_ = function* () {}) {}) (function(_ = { _() {} }) {}) (function(_ = class { static m() {} }) {}) (function([_ = function() {}]) {}) "use strict";(function(_ = function() {}) {}) I'm not sure if this actually qualifies as a vulnerability, but I thought it would be better to err on the side of caution.
,
Apr 16 2016
,
Apr 17 2016
,
Apr 17 2016
,
Apr 18 2016
1, Super easy-to-use repro! Thanks Mike! 2, Bisect suggests this has been introduced with the original implementation of default parameters. (crrev.com/1127063003) 3, Crash is in full-code-gen, when it tries to compile a FunctionLiteral::body() which is nullptr. I'm not sure yet why that would happen, how this interacts with pre-parsing (if at all; it sounds a bit unlikely), and whether this is best fixed by either not having an empty body() or by robust-ifying the code to deal with an empty body(). Crash stack: Note statements=0x0 in VisitStatements. #1 0x0000000000cf60e1 in v8::internal::AstVisitor::VisitStatements ( this=0x7ffda121bcf8, statements=0x0) at ../../src/ast/ast.cc:797 #2 0x0000000000c78319 in v8::internal::FullCodeGenerator::Generate ( this=0x7ffda121bcf8) at ../../src/full-codegen/x64/full-codegen-x64.cc:313 #3 0x00000000008517ed in v8::internal::FullCodeGenerator::MakeCode ( info=0x7ffda121c088) at ../../src/full-codegen/full-codegen.cc:49
,
Apr 18 2016
What I think happens is: - Full-code-gen wants to compile the (outer) function. - Because --min_preparse_length 1, the parser skips (lazy parses) the 'inner' function. (That is, the default value for the parameter.) - The AstVisitor that implements full-code-gen assumes the function is already parsed. That's true for the function it's actually compiling, but not for the FunctionLiteral in the de-sugared default parameter assignment. - Nobody forsaw this, and things get crashy.
,
Apr 18 2016
ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://cluster-fuzz.appspot.com/testcase?key=5766444859523072
,
Apr 18 2016
The repro is predicated on --min_preparse_length 1, but of course the bug would also occur in practice, if only the (inner) function is long enough.
,
Apr 18 2016
ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://cluster-fuzz.appspot.com/testcase?key=6589132238749696
,
Apr 18 2016
ClusterFuzz is analyzing your testcase. Developers can follow the progress at https://cluster-fuzz.appspot.com/testcase?key=5762905001164800
,
Apr 18 2016
Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5762905001164800 Uploader: tsepez@chromium.org Job Type: linux_asan_chrome_mp Platform Id: linux Crash Type: UNKNOWN READ Crash Address: 0x00000000000c Crash State: v8::internal::AstVisitor::VisitStatements v8::internal::FullCodeGenerator::Generate v8::internal::FullCodeGenerator::MakeCode Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_chrome_mp&range=172836:173286 Minimized Testcase (1.02 Kb): Download: https://cluster-fuzz.appspot.com/download/AMIfv94DPB0M39a03v2Nt24xDTVGI9LLtTfFcIQaiEWyHIoFupTDuP6crSbd_G2oIIxPBLe_AW5G_VFDOS6lkP_ItKxFStj_gK0z7wOPQctmo54dPKe05-QFkZVpLDpgj3QOYRTYn_6tSWf3BoIZWeddWy01fmmw8g <script> //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (function(_=function() {}) {}) </script> See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
,
Apr 18 2016
,
Apr 18 2016
Looks to be a fixed-offset null deref, which is not a security issue.
,
Apr 19 2016
After a bit more debugging + discussions: #6 was wrong What happens in the crash is that the 'inner' function literal is parsed just fine, but the outer one is lazy parsed and then compilation fails. The cause is the parenthesized_function_ flag, which is set earlier in the parser and stored on the Parser object. Since ParseFunctionLiteral reads this *after* parsing the parameters, we can run in the situation where the ParseFunctionLiteral is caused recursively, and the 'inner' function literal receives the parenthesized_function_ flag meant for the outer one (and vice versa). This is easily solved by storing it in a local variable, before parsing the function params. Added a regression test & a hopefully readable comment.
,
Apr 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/ed9b7d92e7ddb80e51dd512614261bc3f70e1747 commit ed9b7d92e7ddb80e51dd512614261bc3f70e1747 Author: vogelheim <vogelheim@chromium.org> Date: Wed Apr 20 09:33:01 2016 Prevent un-parsed LiteralFunction reaching the compiler. BUG= chromium:604044 LOG=Y Review URL: https://codereview.chromium.org/1895123002 Cr-Commit-Position: refs/heads/master@{#35650} [modify] https://crrev.com/ed9b7d92e7ddb80e51dd512614261bc3f70e1747/src/parsing/parser-base.h [modify] https://crrev.com/ed9b7d92e7ddb80e51dd512614261bc3f70e1747/src/parsing/parser.cc [modify] https://crrev.com/ed9b7d92e7ddb80e51dd512614261bc3f70e1747/src/parsing/preparser.cc [add] https://crrev.com/ed9b7d92e7ddb80e51dd512614261bc3f70e1747/test/mjsunit/regress-604044.js
,
Apr 20 2016
,
Apr 21 2016
ClusterFuzz has detected this issue as fixed in range 388479:388495. Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5762905001164800 Uploader: tsepez@chromium.org Job Type: linux_asan_chrome_mp Platform Id: linux Crash Type: UNKNOWN READ Crash Address: 0x00000000000c Crash State: v8::internal::AstVisitor::VisitStatements v8::internal::FullCodeGenerator::Generate v8::internal::FullCodeGenerator::MakeCode Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_chrome_mp&range=172836:173286 Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_asan_chrome_mp&range=388479:388495 Minimized Testcase (1.02 Kb): Download: https://cluster-fuzz.appspot.com/download/AMIfv94DPB0M39a03v2Nt24xDTVGI9LLtTfFcIQaiEWyHIoFupTDuP6crSbd_G2oIIxPBLe_AW5G_VFDOS6lkP_ItKxFStj_gK0z7wOPQctmo54dPKe05-QFkZVpLDpgj3QOYRTYn_6tSWf3BoIZWeddWy01fmmw8g <script> //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx //xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx (function(_=function() {}) {}) </script> See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information. If you suspect that the result above is incorrect, try re-doing that job on the test case report page. |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by m...@mikepennisi.com
, Apr 15 20161.0 KB
1.0 KB View Download