CID 1136431: Out-of-bounds access as found by Coverity

RESOLVED INVALID

Status

()

RESOLVED INVALID
4 years ago
3 months ago

People

(Reporter: gkw, Unassigned)

Tracking

(Blocks: 1 bug, {coverity})

Trunk
coverity
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

4 years ago
+++ This bug was initially created as a clone of Bug #1037890 +++

Coverity analysis of source code in js/src has found an Out-of-bounds access.

146bool
147CheckOverRecursedWithExtra(JSContext *cx, BaselineFrame *frame,
148                           uint32_t extra, uint32_t earlyCheck)
149{
150    JS_ASSERT_IF(earlyCheck, !frame->overRecursed());
151
152    // See |CheckOverRecursed| above.  This is a variant of that function which
153    // accepts an argument holding the extra stack space needed for the Baseline
154    // frame that's about to be pushed.
155    uint8_t spDummy;
   
1. address_of: Taking address with &spDummy yields a singleton pointer.
   
CID 1136431 (#1 of 1): Out-of-bounds access (ARRAY_VS_SINGLETON)2. ptr_arith: Using &spDummy as an array. This might corrupt or misinterpret adjacent memory locations.
156    uint8_t *checkSp = (&spDummy) - extra;
157    if (earlyCheck) {
158#if defined(JS_ARM_SIMULATOR) || defined(JS_MIPS_SIMULATOR)
159        (void)checkSp;
160        JS_CHECK_SIMULATOR_RECURSION_WITH_EXTRA(cx, extra, frame->setOverRecursed());
161#else
162        JS_CHECK_RECURSION_WITH_SP(cx, checkSp, frame->setOverRecursed());
163#endif
164        return true;
165    }

in file js/src/jit/VMFunctions.cpp .

Jan, any thoughts on how to move forward here? (not sure how bad this is, so setting s-s first.)
Flags: needinfo?(jdemooij)
Group: javascript-core-security
I don't think this is a real issue but forwarding to Kannan.
Flags: needinfo?(jdemooij) → needinfo?(kvijayan)
This is a bogus assert.  Yes, we are taking the address of an unitialized stack address, and then treating it as a pointer-to-array, and then subtracting from it.  But this is all purely for address bounds checking and does not involve dereferencing the array anywhere.

It doesn't seem from coverity's description of the problem that it is actually detecting a bad memory access.  It's just getting suspicious over using a pointer to an unititialized stack slot for arithmetic.

For good measure I doublechecked the call paths referenced in the report, and there are no memory accesses being done on this.  Is there some pragma we can use to tell coverity to ignore its judgements inside a piece of code?

Marking this INVALID.
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Flags: needinfo?(kvijayan)
Resolution: --- → INVALID
Group: core-security
Blocks: 1230156
You need to log in before you can comment on or make changes to this bug.