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

[Andrew Hughes]

----- Original Message -----
> Andrew et al,
> 
> AFAICS here:
> 
>        220     encoding_variant = malloc(strlen(temp)+1);
>        221     if (encoding_variant == NULL) {
>        222         JNU_ThrowOutOfMemoryError(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.

But I can add it if necessary.  It's trivial after all and does not real
harm.

> 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