nsTextFrame.cpp::PaintAsciiText may use uninitialized variable

RESOLVED INVALID

Status

()

Firefox
General
RESOLVED INVALID
12 years ago
12 years ago

People

(Reporter: georgi - hopefully not receiving bugspam, Unassigned)

Tracking

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

nsTextFrame.cpp::PaintAsciiText may use uninitialized variable

1) http://lxr.mozilla.org/seamonkey/source/layout/generic/nsTextFrame.cpp#3834
const char* text;
at this point |text| may point to anything depending on previous stack content.

2) http://lxr.mozilla.org/seamonkey/source/layout/generic/nsTextFrame.cpp#3888
if this branch is hit, |text| is uninitialized and may point to anything.

later in this method |text| is used at least for reading, but if it is used
for writing this may be exploitable.

so if 2) is hit, at least a crash may be guaranteed
1) http://lxr.mozilla.org/seamonkey/source/layout/generic/nsTextFrame.cpp#3834
const char* text;
making the above
const char* text=0;

will give deterministic behavior imho (most oses don't start mapping from 0).

building on linux with gcc -Wuninitialized gives 133 warnings - some seem harmless, but some like this are not clear.

since uninitialized variables are exploitable, probably the warnings should be shut.
Except that |text| is only used if |textLength > 0|, which will won't be the case in the only case where |text| is uninitialized, right?
(In reply to comment #2)
> Except that |text| is only used if |textLength > 0|, which will won't be the
> case in the only case where |text| is uninitialized, right?
> 

yes, it seems like so. i have missed that condition, so this bug is invalid.

though some of the rest of uninitiliazed gcc warnings are more complex and depend on inter method conditions, so it is very difficult for me to tell value reachability. 
http://lxr.mozilla.org/seamonkey/source/content/svg/content/src/nsISVGPathFlatten.h
86   float Length() {
 87     float xx, yy, length = 0;
 88     for (PRUint32 i = 0; i < count; i++) {
 89       if (type[i] == NS_SVGPATHFLATTEN_LINE) {
 90         float dx = x[i] - xx;
 91         float dy = y[i] - yy;
 92         length += sqrt(dx*dx + dy*dy);
 93       }
 94       xx = x[i];
 95       yy = y[i];
 96     }
 97     return length;
 98   }
 99 };

in this case isn't very clear if |type[0] == NS_SVGPATHFLATTEN_LINE| may be true
This bug is morphing, but the SVG guys might want to take a look at comment 4.
(In reply to comment #4)
> http://lxr.mozilla.org/seamonkey/source/content/svg/content/src/nsISVGPathFlatten.h
> 86   float Length() {
>  87     float xx, yy, length = 0;
>  88     for (PRUint32 i = 0; i < count; i++) {
>  89       if (type[i] == NS_SVGPATHFLATTEN_LINE) {
>  90         float dx = x[i] - xx;
>  91         float dy = y[i] - yy;
>  92         length += sqrt(dx*dx + dy*dy);
>  93       }
>  94       xx = x[i];
>  95       yy = y[i];
>  96     }
>  97     return length;
>  98   }
>  99 };
> 
> in this case isn't very clear if |type[0] == NS_SVGPATHFLATTEN_LINE| may be
> true
> 

this seems fixed by Bug 333631

Indeed. Sorry I missed comment 4.

Closing as invalid since the original issue was a false positive.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → INVALID
Group: security
You need to log in before you can comment on or make changes to this bug.