Closed Bug 1300504 Opened 3 years ago Closed 3 years ago

[Static Analysis][Dereference null return value] In function NewBuiltinInstanceMethodCall

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: andi, Assigned: andi)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: CID 1372413)

Attachments

(1 file)

The Static Analysis tool Coverity detected that variable |call| gets allocated memory and after that is used without being null checked.
The places where null is returned in MWasmCall::New is:

>>    if (!call->argRegs_.init(alloc, args.length()))
>>        return nullptr;

and 

>>    if (!call->init(alloc, call->argRegs_.length() + (callee.isTable() ? 1 : 0)))
>>        return nullptr;

We can see that the return of MWasmCall::New is checked in several occasions across our code like: WasmIonCompile.cpp

>>        auto* ins = MWasmCall::New(alloc(), desc, callee, call.regArgs_, ToMIRType(ret),
>>                                   call.spIncrement_, call.tlsStackOffset_);
>>        if (!ins)
>>            return false;
Comment on attachment 8788127 [details]
Bug 1300504 - prevent null pointer dereference in MWasmCall::NewBuiltinInstanceMethodCall.

https://reviewboard.mozilla.org/r/76710/#review74784

Thanks! r=me with nits addressed.

::: js/src/jit/MIR.cpp:5435
(Diff revision 1)
>      auto callee = wasm::CalleeDesc::builtinInstanceMethod(builtin);
>      MWasmCall* call = MWasmCall::New(alloc, desc, callee, args, resultType, spIncrement,
>                                       MWasmCall::DontSaveTls, nullptr);
>  
> +    if (!call) {
> +      return nullptr;

Nit: no braces and 4 space indent for the return statement. I'd also remove the blank line before the if-statement.
Attachment #8788127 - Flags: review+
Attachment #8788127 - Flags: review?(jorendorff)
Pushed by bpostelnicu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/39c055047869
prevent null pointer dereference in MWasmCall::NewBuiltinInstanceMethodCall. r=jandem
https://hg.mozilla.org/mozilla-central/rev/39c055047869
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
You need to log in before you can comment on or make changes to this bug.