Closed Bug 615706 Opened 10 years ago Closed 9 years ago

Possible integer overflow in nsUnicodeToJamoTTF.cpp with newer gcc versions


(Core :: Internationalization, defect)

Not set



Tracking Status
blocking2.0 --- betaN+


(Reporter: josh, Assigned: smontagu)


(Whiteboard: [sg:critical?])


(1 file)

Newer gcc versions contain certain optimizations that assume signed integer arithmetic will never overflow.

Inside nsUnicodeToJamoTTF.cpp we have the ScanDecomposeSyllable function. This contains this code:

nsresult ScanDecomposeSyllable(PRUnichar* aIn, PRInt32 *aLength, 
                               const PRInt32 maxLength)
  nsresult rv = NS_OK;

  if (!aIn || *aLength < 1 || maxLength < *aLength + 2)

This function is called as:
ScanDecomposeSyllable(*aOutSeq, aLength, *aLength + 4);

So our arithmetic ends up being
    *aLength + 4 < *aLength + 2

This should always be true, unless aLength is going to wrap when 4 is added to it. gcc optimizes this check away though, according to the warning generated during compile:

nsUnicodeToJamoTTF.cpp:876:3: warning: assuming signed overflow does not occur when assuming that (X + c) < X is always false

I'm not sure if this can be exploited to do anything dangerous, I'm not familiar enough with the code, nor do I plan to spend hours figuring this out.

I'll leave it up to you folks to decide if this should be treated as a security issue. It is however a bug and should be fixed.
Marking this sg:critical?, but Simon, if you feel this is not exploitable please let us know.
blocking2.0: --- → betaN+
Whiteboard: [sg:critical?]
nsUnicodeToJamoTTF seems to be unused anyway. I guess it fell through the cracks when we removed the obsolete font encoders (in bug 413613) because it was used in the old gfx/windows code and not just for X like the rest of the font encoders.
Attachment #495967 - Flags: review?(VYV03354)
Attachment #495967 - Flags: review?(VYV03354) → review+
Keywords: checkin-needed
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.