Closed
Bug 1273152
Opened 8 years ago
Closed 8 years ago
[Static Analysis][Dereference after null check] In code generated by Codegen.py
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
INVALID
Tracking | Status | |
---|---|---|
firefox49 | --- | affected |
People
(Reporter: andi, Assigned: andi)
References
(Blocks 1 open bug)
Details
(Keywords: coverity, Whiteboard: CID 1361628, CID 1361627, CID 1361626, CID 1361624, CID 1361623, CID 1361622 btpp-active)
Attachments
(1 file)
the code generated by Codegen.py in CGDictionary(CGThing) function can produce a cpp file that during execution can lead to a null pointer dereference even though it was previously null checked. As follows: >> if self.needToInitIds: >> body += fill( >> """ >> ${dictName}Atoms* atomsCache = nullptr; >> if (cx) { >> atomsCache = GetAtomCache<${dictName}Atoms>(cx); >> if (!*reinterpret_cast<jsid**>(atomsCache) && !InitIds(cx, atomsCache)) { >> return false; >> } >> } >> >> """, >> dictName=self.makeClassName(self.dictionary)) >> >> if self.dictionary.parent: >> body += fill( >> """ >> // Per spec, we init the parent's members first >> if (!${dictName}::Init(cx, val)) { >> return false; >> } >> >> """, >> dictName=self.makeClassName(self.dictionary.parent)) >> else: >> body += dedent( >> """ >> { // scope for isConvertible >> bool isConvertible; >> if (!IsConvertibleToDictionary(cx, val, &isConvertible)) { >> return false; >> } >> if (!isConvertible) { >> return ThrowErrorMessage(cx, MSG_NOT_DICTIONARY, sourceDescription); >> } >> } >> >> """) As an example we can take function MediaKeySystemConfiguration::Init from MediaKeySystemAccessBinding.cpp >> if (cx) { >> atomsCache = GetAtomCache<MediaKeySystemConfigurationAtoms>(cx); >> if (!*reinterpret_cast<jsid**>(atomsCache) && !InitIds(cx, atomsCache)) { >> return false; >> } >> } >> >> { // scope for isConvertible >> bool isConvertible; >> if (!IsConvertibleToDictionary(cx, val, &isConvertible)) { >> return false; >> } >> if (!isConvertible) { >> return ThrowErrorMessage(cx, MSG_NOT_DICTIONARY, sourceDescription); >> } >> } I think we should modify the py file in order to prevent null dereferences.
Assignee | ||
Comment 1•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/52834/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/52834/
Attachment #8752853 -
Flags: review?(jst)
Comment 2•8 years ago
|
||
Comment on attachment 8752853 [details] MozReview Request: Bug 1273152 - modify autogen python to prevent null pointer dereference on generates cpps. r?jst Redirecting to peterv. We're at the very least inconsistent in where we check for cx being null in the generated code... One question I have is whether we should just check once rather than checking separately in the 3 places where I saw the checks being done in a single sample of actual generated code. Regardless, peterv knows this far better than I do.
Attachment #8752853 -
Flags: review?(jst) → review?(peterv)
Updated•8 years ago
|
Whiteboard: CID 1361628, CID 1361627, CID 1361626, CID 1361624, CID 1361623, CID 1361622 → CID 1361628, CID 1361627, CID 1361626, CID 1361624, CID 1361623, CID 1361622 btpp-active
Comment 3•8 years ago
|
||
Comment on attachment 8752853 [details] MozReview Request: Bug 1273152 - modify autogen python to prevent null pointer dereference on generates cpps. r?jst The code is fine as is, it even has comments about a null cx being ok if we're initing from null.
Attachment #8752853 -
Flags: review?(peterv) → review-
Comment 4•8 years ago
|
||
To expand on that, IsConvertibleToDictionary doesn't use cx if val is a null JS::Value.
Assignee | ||
Updated•8 years ago
|
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → INVALID
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•