1361509 - coverity report: Non-static class member "mSource" is not initialized in this constructor nor in any functions that it calls.

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Mats Palmgren (inactive)]

Filing this as security sensitive just in case...
At first glance I can't find anyone initializing nsSVGGradientFrame::mSource.


** CID 1405753:  Uninitialized members  (UNINIT_CTOR)
/layout/svg/nsSVGGradientFrame.cpp: 34 in nsSVGGradientFrame::nsSVGGradientFrame(nsStyleContext *, mozilla::FrameType)()


________________________________________________________________________________________________________
*** CID 1405753:  Uninitialized members  (UNINIT_CTOR)
/layout/svg/nsSVGGradientFrame.cpp: 34 in nsSVGGradientFrame::nsSVGGradientFrame(nsStyleContext *, mozilla::FrameType)()
28     nsSVGGradientFrame::nsSVGGradientFrame(nsStyleContext* aContext,
29                                            FrameType aType)
30       : nsSVGPaintServerFrame(aContext, aType)
31       , mLoopFlag(false)
32       , mNoHRefURI(false)
33     {
>>>     CID 1405753:  Uninitialized members  (UNINIT_CTOR)
>>>     Non-static class member "mSource" is not initialized in this constructor nor in any functions that it calls.
34     }
35     
36     //----------------------------------------------------------------------
37     // nsIFrame methods:
38     
39     nsresult

Comment 1

[Mats Palmgren (inactive)]

I think this is sec-low at worst since it's a frame class allocated in the pres arena,
so there's no type confusion here.  Either mSource is null, or it points to a frame
that may or may not be alive but that frame is always allocated in the same arena,
so frame-poisoning will save us from being exploited.

Comment 2

[Robert Longson [:longsonr]]

https://dxr.mozilla.org/mozilla-central/source/layout/svg/nsSVGGradientFrame.cpp#243

how do we mark false positives?

Comment 3

[Mats Palmgren (inactive)]

Could we initialize this member to nullptr in the ctor anyway, please?
Even if there *currently* is a path that init mSource before it's used,
it seems safer to always init it in the ctor.  Also, it shuts up Coverity
so that we don't have to look at this one again. :-)

Comment 4

[Robert Longson [:longsonr]]

I've no objections to doing that.

Comment 5

[Robert Longson [:longsonr]]

Attachment: patch

Comment 6

[Robert Longson [:longsonr]]

Do I need any additional approval to land this?

Comment 7

[Mats Palmgren (inactive)]

Comment on attachment 8865669 [details] [diff] [review]
patch

Thanks.

> Do I need any additional approval to land this?

No, only sec-high/sec-critical needs sec-approval.
https://wiki.mozilla.org/Security/Bug_Approval_Process

Comment 8

[Robert Longson [:longsonr]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f804bb28ab999e3ae19ff5e7039f58674bbe9632

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f804bb28ab99