Closed Bug 615706 Opened 10 years ago Closed 9 years ago

Possible integer overflow in nsUnicodeToJamoTTF.cpp with newer gcc versions

Categories

(Core :: Internationalization, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- betaN+

People

(Reporter: josh, Assigned: smontagu)

Details

(Whiteboard: [sg:critical?])

Attachments

(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)
    return NS_ERROR_INVALID_ARG;


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
http://hg.mozilla.org/mozilla-central/rev/e51f0055becf
Status: NEW → RESOLVED
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.