Closed Bug 1300518 Opened 3 years ago Closed 3 years ago

[Static Analysis][Dereference after null check] In function nsCSSFrameConstructor::IsValidSibling

Categories

(Core :: Layout, 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 1372468)

Attachments

(1 file)

The Static Analysis tool Coverity detected that a null pointer dereference may happen in the following context for |parentFrame|:

>>  if (IsFrameForFieldSet(parentFrame, parentType)) {
>>    // Legends can be sibling of legends but not of other content in the fieldset
>>    if (nsContainerFrame* cif = aSibling->GetContentInsertionFrame()) {
>>      aSibling = cif;
>>    }

|parentFrame| is null checked but dereference occurs outside of the null check scope:

>>  if (parentFrame) {
>>    parentType = parentFrame->GetType();
>>  }
Comment on attachment 8788144 [details]
Bug 1300518 - removed nullcheck for parentFrame in nsCSSFrameConstructor::IsValidSibling.

https://reviewboard.mozilla.org/r/76740/#review74844

Viewing the only callsite of this function, I believe parentFrame would never be nullptr... The code checking parentFrame was introduced in bug 403733, while parentFrame->GetType() preexists that check, and the check was added there without any justification. I guess we can try removing the check of parentFrame directly rather than adding more check.
Attachment #8788144 - Flags: review?(xidorn+moz)
Comment on attachment 8788144 [details]
Bug 1300518 - removed nullcheck for parentFrame in nsCSSFrameConstructor::IsValidSibling.

https://reviewboard.mozilla.org/r/76740/#review74848
Attachment #8788144 - Flags: review?(xidorn+moz) → review+
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6e805e28151d
removed nullcheck for parentFrame in nsCSSFrameConstructor::IsValidSibling. r=xidorn
https://hg.mozilla.org/mozilla-central/rev/6e805e28151d
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.