Closed Bug 1338270 Opened 3 years ago Closed 3 years ago

apparent bounds violations in jsapi-tests/testGCExactRooting.cpp

Categories

(Core :: JavaScript: GC, defect)

45 Branch
defect
Not set

Tracking

()

RESOLVED INVALID

People

(Reporter: jesup, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: csectype-bounds, sec-other, Whiteboard: CID 1345643)

sec-other since these are only tests.

From coverity:

________________________________________________________________________________________________________
*** CID 1345643:  Memory - corruptions  (OVERRUN)
/js/src/jsapi-tests/testGCExactRooting.cpp: 237 in cls_testGCRootedVector::run(JS::Handle<JSObject *>)()
231             RootedValue val(cx, UndefinedValue());
232             // Construct a unique property name to ensure that the object creates a
233             // new shape.
234             char buffer[2];
235             buffer[0] = 'a' + i;
236             buffer[1] = '\0';
>>>     CID 1345643:  Memory - corruptions  (OVERRUN)
>>>     Overrunning array "buffer" of 2 bytes by passing it to a function which accesses it at byte offset 2.
237             CHECK(JS_SetProperty(cx, obj, buffer, val));
238             CHECK(shapes.append(obj->as<NativeObject>().lastProperty()));
239         }
240     
241         JS_GC(cx);
242         JS_GC(cx);

** CID 1345644:  Memory - corruptions  (OVERRUN)


________________________________________________________________________________________________________
*** CID 1345644:  Memory - corruptions  (OVERRUN)
/js/src/jsapi-tests/testGCExactRooting.cpp: 160 in cls_testGCRootedHashMap::run(JS::Handle<JSObject *>)()
154             RootedValue val(cx, UndefinedValue());
155             // Construct a unique property name to ensure that the object creates a
156             // new shape.
157             char buffer[2];
158             buffer[0] = 'a' + i;
159             buffer[1] = '\0';
>>>     CID 1345644:  Memory - corruptions  (OVERRUN)
>>>     Overrunning array "buffer" of 2 bytes by passing it to a function which accesses it at byte offset 2.
160             CHECK(JS_SetProperty(cx, obj, buffer, val));
161             CHECK(map.putNew(obj->as<NativeObject>().lastProperty(), obj));
162         }
163     
164         JS_GC(cx);
165         JS_GC(cx);

** CID 1345645:  Memory - corruptions  (OVERRUN)


________________________________________________________________________________________________________
*** CID 1345645:  Memory - corruptions  (OVERRUN)
/js/src/jsapi-tests/testGCExactRooting.cpp: 309 in cls_testTraceableFifo::run(JS::Handle<JSObject *>)()
303             RootedValue val(cx, UndefinedValue());
304             // Construct a unique property name to ensure that the object creates a
305             // new shape.
306             char buffer[2];
307             buffer[0] = 'a' + i;
308             buffer[1] = '\0';
>>>     CID 1345645:  Memory - corruptions  (OVERRUN)
>>>     Overrunning array "buffer" of 2 bytes by passing it to a function which accesses it at byte offset 2.
309             CHECK(JS_SetProperty(cx, obj, buffer, val));
310             CHECK(shapes.pushBack(obj->as<NativeObject>().lastProperty()));
311         }
312     
313         CHECK(shapes.length() == 10);
314     

** CID 1345646:  Memory - corruptions  (OVERRUN)


________________________________________________________________________________________________________
*** CID 1345646:  Memory - corruptions  (OVERRUN)
/js/src/jsapi-tests/testGCExactRooting.cpp: 187 in FillMyHashMap(JSContext *, JS::MutableHandle<JS::GCHashMap<js::Shape *, JSObject *, js::DefaultHasher<js::Shape *>, js::TempAllocPolicy, JS::DefaultMapSweepPolicy<js::Shape *, JSObject *>>>)()
181             RootedValue val(cx, UndefinedValue());
182             // Construct a unique property name to ensure that the object creates a
183             // new shape.
184             char buffer[2];
185             buffer[0] = 'a' + i;
186             buffer[1] = '\0';
>>>     CID 1345646:  Memory - corruptions  (OVERRUN)
>>>     Overrunning array "buffer" of 2 bytes by passing it to a function which accesses it at byte offset 2.
187             if (!JS_SetProperty(cx, obj, buffer, val))
188                 return false;
189             if (!map.putNew(obj->as<NativeObject>().lastProperty(), obj))
190                 return false;
191         }
192         return true;

** CID 1345647:  Memory - corruptions  (OVERRUN)


________________________________________________________________________________________________________
*** CID 1345647:  Memory - corruptions  (OVERRUN)
/js/src/jsapi-tests/testGCExactRooting.cpp: 347 in FillVector(JSContext *, JS::MutableHandle<JS::GCVector<js::Shape *, (unsigned long)0, js::TempAllocPolicy>>)()
341             RootedValue val(cx, UndefinedValue());
342             // Construct a unique property name to ensure that the object creates a
343             // new shape.
344             char buffer[2];
345             buffer[0] = 'a' + i;
346             buffer[1] = '\0';
>>>     CID 1345647:  Memory - corruptions  (OVERRUN)
>>>     Overrunning array "buffer" of 2 bytes by passing it to a function which accesses it at byte offset 2.
347             if (!JS_SetProperty(cx, obj, buffer, val))
348                 return false;
349             if (!shapes.append(obj->as<NativeObject>().lastProperty()))
350                 return false;
351         }
352
(In reply to Randell Jesup [:jesup] from comment #0)
It seems that it's complaining that JS_SetProperty is accessing past the end of the array.  Is there any way to find out where it thinks that happens?  I checked the code and it seems fine.
Flags: needinfo?(rjesup)
Component: JavaScript Engine → JavaScript: GC
I tried to look up the CID report, but I couldn't find it.
Whiteboard: CID 1345643
jonco
Flags: needinfo?(rjesup)
(In reply to Jon Coppeard (:jonco) from comment #1)
> (In reply to Randell Jesup [:jesup] from comment #0)
> It seems that it's complaining that JS_SetProperty is accessing past the end
> of the array.  Is there any way to find out where it thinks that happens?  I
> checked the code and it seems fine.

jonco: no idea, I just see the reports - christoph - who's handling this stuff?
Flags: needinfo?(cdiehl)
I've fixed a lot of coverity issues so if anybody needs my help with this one, just ping me.
I am not doing any JS security/fuzzing Randell. I forward your needinfo to Christian instead may be he can help out.
Flags: needinfo?(cdiehl) → needinfo?(choller)
(In reply to Andi-Bogdan Postelnicu from comment #5)
Hi, do you know the answer to the question in comment 1?  Can we find our where coverity thinks the overrun is happening?
Flags: needinfo?(bpostelnicu)
Canceling needinfo because JS developers already seem to be on this one and I can't add much more because I don't usually work with Coverity.
Flags: needinfo?(choller)
(In reply to Jon Coppeard (:jonco) from comment #7)
> (In reply to Andi-Bogdan Postelnicu from comment #5)
> Hi, do you know the answer to the question in comment 1?  Can we find our
> where coverity thinks the overrun is happening?

I can't find the ids in the coverity db for the firefox project. I even looked in the snapshot from that date, and indeed they seem to be present in the sent email but they are not present in the coverity db report, so there is no way on how we can investigate this further without having access to the report. 
Anyway dealing with the info that we have let's take for example cid 1345643, we see that buffer, being 2 bytes and having it's strlen = 1 it's passed to JS_SetProperty thus decaying it to a pointer. And more if we follow the calls that are performed onwards we see that pointer is passed correctly, without manipulating it's original address, by increment or decrement. 
I strongly believe that this is false-positive.
Flags: needinfo?(bpostelnicu)
Closing as invalid as this appears to be a false positive.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.