Closed Bug 991353 Opened 6 years ago Closed 6 years ago

CID 1191020: Use after free in vm/PIC.cpp as found by Coverity

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set

Tracking

()

RESOLVED INVALID

People

(Reporter: gkw, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: coverity)

Coverity analysis of source code in js/src has found an Use after free issue in vm/PIC.cpp.


250// Trace the pointers stored directly on the stub.
251void
252js::ForOfPIC::Chain::mark(JSTracer *trc)
253{
   
1. Condition "!this->initialized_", taking false branch
   
2. Condition "this->disabled_", taking false branch
254    if (!initialized_ || disabled_)
255        return;
256
257    gc::MarkObject(trc, &arrayProto_, "ForOfPIC Array.prototype.");
258    gc::MarkObject(trc, &arrayIteratorProto_, "ForOfPIC ArrayIterator.prototype.");
259
260    gc::MarkShape(trc, &arrayProtoShape_, "ForOfPIC Array.prototype shape.");
261    gc::MarkShape(trc, &arrayIteratorProtoShape_, "ForOfPIC ArrayIterator.prototype shape.");
262
263    gc::MarkValue(trc, &canonicalIteratorFunc_, "ForOfPIC ArrayValues builtin.");
264    gc::MarkValue(trc, &canonicalNextFunc_, "ForOfPIC ArrayIterator.prototype.next builtin.");
265
266    // Free all the stubs in the chain.
   
3. Condition "this->stubs_", taking true branch
   
6. Condition "this->stubs_", taking true branch
267    while (stubs_)
   
4. freed_arg: "js::PICChain<js::ForOfPIC>::removeStub(js::PICChain<js::ForOfPIC>::CatStub *, js::PICChain<js::ForOfPIC>::CatStub *)" frees "this->stubs_". [show details]
   
5. Jumping back to the beginning of the loop
   
CID 1191020 (#1 of 1): Use after free (USE_AFTER_FREE)7. deref_arg: Calling "js::PICChain<js::ForOfPIC>::removeStub(js::PICChain<js::ForOfPIC>::CatStub *, js::PICChain<js::ForOfPIC>::CatStub *)" dereferences freed pointer "this->stubs_". [show details]
268        removeStub(stubs_, nullptr);
269}

s-s pending analysis because this is a UAF found by a static analyzer.

Jan, any idea how to move forward?
Flags: needinfo?(jdemooij)
(not sure if this is a JIT / interpreter bug)
Forwarding to Kannan.
Flags: needinfo?(jdemooij) → needinfo?(kvijayan)
This is a false flag. In the case where we pass |stubs_| into |removeStub()|, it overwrites stubs_ before freeing it.  This is standard "eat-from-front" loop-deleting behaviour.

How do we get coverity to stop triggering on this case?
Flags: needinfo?(kvijayan)
> How do we get coverity to stop triggering on this case?

We'd have to create a modeling file that will suppress this in Coverity.

In the meantime, this becomes INVALID. Thanks for looking at this!
Group: javascript-core-security
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → INVALID
Whiteboard: [CID 1191020]
Summary: Use after free in vm/PIC.cpp as found by Coverity → CID 1191020: Use after free in vm/PIC.cpp as found by Coverity
Whiteboard: [CID 1191020]
You need to log in before you can comment on or make changes to this bug.