Request for approval: Bug 100057 (original) (raw)

Request for approval: Bug 100057 - Potential stack corruption in GetJavaProperties

David Holmes - Sun Microsystems David.Holmes at Sun.COM
Fri May 22 11:58:35 UTC 2009

Andrew Haley said the following on 05/22/09 21:45:

David Holmes - Sun Microsystems wrote:

If you use malloc then you have to check for a NULL return and deal with the error possibility.

Alternatively use strncpy to make sure it's safe and continue to assume that it will be big enough. It's just following the style used throughout that file, which is to allocate memory (usually via strdup) but not check the retval.

Hmmm. In most (all?) cases though that just results in a NULL being stored in a property. Here if we get NULL we try to write into that space - and that's not good. Can't say for sure all the strdup uses are safe though.

The risk of fixing old code is that suddenly people notice everything else that's wrong with it.

OK. I'll check for the NULL, then. If I have to change the patch that's been in IcedTea for ages then I'll use strdup instead of malloc.

I do wonder why strdup wasn't used in the first place.

It also appears that the strdup of lc is never freed.

But what is one supposed to do if the allocation fails? Simply emit an error message to stderr and call abort() ?

Throw an OutOfMemory exception - see the use of JNU_ThrowByName later in the function.

Cheers, David

Andrew.

Andrew Haley said the following on 05/22/09 21:10: https://bugs.openjdk.java.net/showbug.cgi?id=100057

GetJavaProperties has a stack-allocated fixed size buffer for holding a copy of a string returned by setlocale(3). However, there is no guarantee that the string will fit into this buffer. This one is probably due to Solaris code being reused for Linux. The patch has been in IcedTea for a long while. OK to push, OpenJDK 7 and 6? Andrew.

--- oldopenjdk6/jdk/src/solaris/native/java/lang/javapropsmd.c 2008-08-28 04:15:51.000000000 -0400 +++ openjdk/jdk/src/solaris/native/java/lang/javapropsmd.c 2008-09-15 10:37:26.000000000 -0400 @@ -211,7 +211,9 @@ * .@ * , , and are optional. */ - char temp[64]; + char * temp; + temp = (char*) malloc(strlen(lc)+1); + char *language = NULL, *country = NULL, *variant = NULL, *encoding = NULL; char *stdlanguage = NULL, *stdcountry = NULL, *stdvariant = NULL, @@ -323,6 +325,9 @@ /* return same result nllanginfo would return for enUK, * in order to use optimizations. */ stdencoding = (*p != '\0') ? p : "ISO8859-1"; + + /* Free temp */ + free(temp); #ifdef linux

More information about the core-libs-dev mailing list