615706 - Possible integer overflow in nsUnicodeToJamoTTF.cpp with newer gcc versions

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Josh Bressers]

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.

Comment 1

[Johnny Stenback (:jst)]

Marking this sg:critical?, but Simon, if you feel this is not exploitable please let us know.

Comment 2

[Simon Montagu :smontagu]

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.

Comment 3

[Simon Montagu :smontagu]

Attachment: Remove the encoder

Comment 4

[Simon Montagu :smontagu]

http://hg.mozilla.org/mozilla-central/rev/e51f0055becf