1425826 - [Static Analysis] Dereference of null return value in BinASTParser::parseBlockStatementAux

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P1)

Description

[Andi [:andi]]

A possible null pointer dereference can occur in the following context:

>>    // In case of absent optional fields, inject default values.
>>    if (!body)
>>        body = factory_.newStatementList(tokenizer_->pos());
>>
>>   MOZ_TRY_VAR(body, appendDirectivesToBody(body, directives));


I think we can avoid this by packing | newStatementList | call with TRY_DECL.

Comment 1

[Andi [:andi]]

Attachment: Bug 1425826 - prevent null pointer dereference in BinASTParser::parseBlockStatementAux.

Review commit: https://reviewboard.mozilla.org/r/208092/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/208092/

Comment 2

[Jason Orendorff [:jorendorff]]

Comment on attachment 8937418 [details]
Bug 1425826 - prevent null pointer dereference in BinASTParser::parseBlockStatementAux.

https://reviewboard.mozilla.org/r/208092/#review214458

r=me with the one comment addressed.

::: js/src/frontend/BinSource.cpp:338
(Diff revision 1)
>          }
>      }
>  
>      // In case of absent optional fields, inject default values.
>      if (!body)
> -        body = factory_.newStatementList(tokenizer_->pos());
> +        TRY_DECL(body, factory_.newStatementList(tokenizer_->pos()));

This seems like it would be a syntax error. Does it compile?

I think it needs to be `TRY_VAR` instead of `TRY_DECL`.

Comment 3

[Andi [:andi]]

Comment on attachment 8937418 [details]
Bug 1425826 - prevent null pointer dereference in BinASTParser::parseBlockStatementAux.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/208092/diff/1-2/

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4d07f50ba637
prevent null pointer dereference in BinASTParser::parseBlockStatementAux. r=jorendorff

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/4d07f50ba637