206363 - fix a buffer-overrun in Jamo converter

Status: RESOLVED FIXED

(Core :: Internationalization, defect)

Description

[Jungshik Shin]

I found a buffer-overrun in nsUnicodeToJamoTTF.cpp that was introduced during my
rewrite to save some memory.  This leads to a crash in some situations. I'm
going to upload a fix to fill up the hole.

Comment 1

[Jungshik Shin]

Attachment: a patch

In addition to fixing the buffer overrun, I changed the name of a local
variable to be compliant to the naming convention often used in Mozilla.

Comment 2

[Jungshik Shin]

Comment on attachment 123743 [details] [diff] [review]
a patch

Asking r/sr. 
This patch is acutally a one-liner :

   // identify the substring of aIn with values in [aOffset, aOffset + 0x100).
-  while ((aIn[start] & 0xff00) != aOffset) 
+  while (start < origLen && (aIn[start] & 0xff00) != aOffset)
     ++start;

Comment 3

[rbs]

Does this also clear this uncertainty that I had seen there earlier?

  // This should never happen, but it happens under MS Windows, somehow...
  if (mJamoCount > mJamosMaxLength) 
  {
    NS_WARNING("mJamoCount > mJamoMaxLength on entering Convert()");
    Reset();
  }

Comment 4

[Jungshik Shin]

No, it doesn't. Actually, I guess we can safely remove that if-clause. I added
it last October (in my tree) and left it there just to be safe. However, I
haven't since seen warning message  from that spot. Reading the code again, I
don't see how the condition  in the if-clause can be satisfied   (well, neither
could I back then).

Comment 5

[Simon Montagu :smontagu]

Comment on attachment 123743 [details] [diff] [review]
a patch

r=smontagu

Comment 6

[rbs]

Comment on attachment 123743 [details] [diff] [review]
a patch

sr=rbs

Comment 7

[Jungshik Shin]

Comment on attachment 123743 [details] [diff] [review]
a patch

Thanks for r/sr.

This is to fix a buffer overrun bug that leads to a crash on some occasions.
Those who never view web pages with Hangul Jamos won't be affected.

Comment 8

[Asa Dotzler [:asa]]

Comment on attachment 123743 [details] [diff] [review]
a patch

a=asa (on behalf of drivers) for checkin to 1.4.

Comment 9

[Jungshik Shin]

checked in the fix. thank you all.