Closed Bug 1361580 Opened 7 years ago Closed 4 months ago

coverity report: ​nsContinuingTextFrame::mPrevContinuation isn't initialized properly

Categories

(Core :: Layout: Text and Fonts, defect, P3)

53 Branch
defect

Tracking

()

RESOLVED FIXED
122 Branch
Tracking Status
firefox-esr115 --- wontfix
firefox120 --- wontfix
firefox121 --- wontfix
firefox122 --- fixed

People

(Reporter: MatsPalmgren_bugz, Assigned: TYLin)

References

(Blocks 1 open bug, )

Details

(Keywords: coverity, regression, Whiteboard: [CID 750303])

Attachments

(2 files)

Filing this as security sensitive just in case...


Coverity CID 750303 Uninitialized pointer field

The pointer field will point to an arbitrary memory location, any attempt to write may cause corruption.

In nsContinuingTextFrame::​nsContinuingTextFrame(nsStyleContext *): A pointer field is not initialized in the constructor 


4354protected:
4355  explicit nsContinuingTextFrame(nsStyleContext* aContext)
4356    : nsTextFrame(aContext)
    CID 750303 (#1 of 1): Uninitialized pointer field (UNINIT_CTOR)2. uninit_member: Non-static class member mPrevContinuation is not initialized in this constructor nor in any functions that it calls.
4357  {}
4358
    1. member_decl: Class member declaration for mPrevContinuation.
4359  nsTextFrame* mPrevContinuation;
4360};
Actually, there's only one path that create nsContinuingTextFrame:
http://searchfox.org/mozilla-central/rev/abe68d5dad139e376d5521ca1d4b7892e1e7f1ba/layout/base/nsCSSFrameConstructor.cpp#9147
and we always call Init there with a non-null aPrevInFlow (aFrame):
http://searchfox.org/mozilla-central/rev/abe68d5dad139e376d5521ca1d4b7892e1e7f1ba/layout/generic/nsSplittableFrame.cpp#26

Still, it's probably worth adding a mPrevContinuation(nullptr) there.
It seems I can no longer remove the core-security flag.

Dan, can you make this bug public please?
Flags: needinfo?(dveditz)
Group: core-security
Flags: needinfo?(dveditz)
Whiteboard: [CID 750303]
Priority: -- → P3
Severity: normal → S3

In nsContinuingTextFrame::Init() [1], we always call SetPrevInFlow() to
initialize mPrevContinuation, so we are fine. Still, it is better to
initialize the pointer properly before Init().

[1] https://searchfox.org/mozilla-central/rev/d7f837add602d270f2b2958b3ab5206dc85965c0/layout/generic/nsTextFrame.cpp#4230

Assignee: nobody → aethanyc
Status: NEW → ASSIGNED
Pushed by aethanyc@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/55dba745f9b3
Initialize nsContinuingTextFrame::mPrevContinuation. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/662bda9458a8
Move the initialization of nsTextFrame members to where they are declared. r=dholbert
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 122 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: