[PATCH FOR REVIEW] Potential Buffer Overflow in java_props_md.c (original) (raw)

Andrew Hughes ahughes at redhat.com
Thu Aug 2 22:03:12 UTC 2012

----- Original Message -----

Hi Andrew,

On 2/08/2012 7:35 PM, Andrew Hughes wrote: > ----- Original Message ----- >> Andrew et al, >> >> AFAICS here: >> >> 220 encodingvariant = malloc(strlen(temp)+1); >> 221 if (encodingvariant == NULL) { >> 222 JNUThrowOutOfMemoryError(env, NULL); >> 223 return 0; >> 224 } >> >> we also need to do free(temp). Similarly later where we return >> with >> OOM >> due to realloc failure, don't we also need to free what was >> previously >> malloc'd? >> > > I was thinking the same just before committing, but didn't want to > delay > the main fix any further. > > While logically we do need one, I'm not sure it's worthwhile if > we're going > to throw the exception and exit anyway? Will the return even be > reached? > If we're already near enough to OOM that we can't allocate more > memory, > it's unlikely freeing temp is going to get us much, and I presume > it will > be freed anyway when the VM process exists. Are we definitely going to exit on this error? I agree if we're out of memory we're likely to continue to run into problems if we try to continue. But I'd prefer to see proper cleanup rather than making assumptions about the context in which the code is used. > But I can add it if necessary. It's trivial after all and does not > real > harm. It will also prevent us getting flagged with memory-leak warnings/errors from static analysis tools.

This should cover it, I think:

http://cr.openjdk.java.net/~andrew/buffer_overflow/webrev.02/

but it'll need a bug ID.

Thanks, David

>> David >> >> On 2/08/2012 7:18 AM, Andrew Hughes wrote: >>> >>> >>> ----- Original Message ----- >>>> On 01/08/2012 14:52, Andrew Hughes wrote: >>>>> : >>>>> >>>>> >>>>> In any case, there is a Sun bug open for this: >>>>> >>>>> 6844255: Potential stack corruption in GetJavaProperties >>>>> >>>>> Can I take it that I can just get on and push Omair's extended >>>>> version now then, >>>>> with that bug ID? >>>> Yes, go ahead, I should have said that in my mail. >>>> >>> >>> Thanks. >>> >>> Done: >>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-August/010993.html >>> >>> with Omair as author and yourself and I as reviewers. >>> >>>>> Well, the locale can be set be an environment variable, so it >>>>> could >>>>> potentially >>>>> be anything of any length... >>>>> >>>>> The Debian bug posted above has an example, though I couldn't >>>>> replicate it. >>>>> >>>> I couldn't replicate it either and was just curious if anyone >>>> managed >>>> to >>>> demonstrate it. >>>> >>> >>> Yeah, I tend to think it's more potentially exploitable rather >>> than >>> something >>> that's actually been hit. >>> >>>> -Alan. >>>> >>> >>> Thanks, >> >

-- Andrew :)

Free Java Software Engineer Red Hat, Inc. (http://www.redhat.com)

PGP Key: 248BDC07 (https://keys.indymedia.org/) Fingerprint = EC5A 1F5E C0AD 1D15 8F1F 8F91 3B96 A578 248B DC07

More information about the core-libs-dev mailing list