[PATCH FOR REVIEW] Potential Buffer Overflow in java_props_md.c

[Alan Bateman]

On 01/08/2012 12:40, Andrew Hughes wrote:
> java_props_md.c allocates a 64 byte buffer for the return value of setlocale
> on the stack.  However, there appears to be no set limit on the return value:
>
> http://pubs.opengroup.org/onlinepubs/009604499/functions/setlocale.html
>
> and no check in the code to ensure that its length is 63 characters or less
> (allowing for '\0').  While language and country are defined by ISO, I don't
> believe there's any limit on the optional encoding and variant entries.
>
> This patch:
>
> http://cr.openjdk.java.net/~andrew/buffer_overflow/webrev.01/jdk.patch
>
> replaces the allocation with a dynamic buffer based on the length of lc.
>
> Original bug: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=497666
>
> Ok for tl?  If so, can I have a bug ID?
>
Just some history on this one.

Andrew Haley originally brought this up up in 2009 (embarrassing, I 
know), see:

http://mail.openjdk.java.net/pipermail/core-libs-dev/2009-May/001650.html

Then in 2011, Omair tried again, see:

http://mail.openjdk.java.net/pipermail/core-libs-dev/2011-May/006907.html

I reviewed it at the time and gave a thumbs up, it's just it was the 
end-game for 7 at the time and that release was being locked down. I 
asked that it be held back until jdk8 and jdk7u.

Omair's webrev is a larger patch that what you are proposing now, mostly 
handling malloc/realloc failing that we nit-picked in the original 
review. I haven't checked IcedTea to know whether it has the minimal 
patch you are proposing now or whether it has Omair's changes.

In any case, there is a Sun bug open for this:

6844255: Potential stack corruption in GetJavaProperties

Also if you read the old mails then you'll see that we were scratching 
our heads as to an example that would demonstrate the original issue. I 
suspect it may have been something that someone spotted rather than 
someone running with a locale of this length.

-Alan.