HI,

On Mon, Apr 14, 2014 at 10:18:51PM +0200, David Sommerseth wrote:
> Regarding your patch - the PolarSSL stuff
> 
> +char *
> +get_ssl_library_version(void)
> +{
> +    static char polar_version[30];   /* "at least 18 bytes in size" */
> +    version_get_string_full( polar_version );
> +    return polar_version;
> +}
> 
> version_get_string_full() is crappy due to the "at least 18 bytes".
> That's ugly.  Can we please ensure that we at least adds a NULL
> termination in polar_version[29] after version_get_string_full() ...
> just to reduce the risk somewhat, in case it suddenly gets longer.  Then
> it should (hopefully) be somewhat harder to get a buffer information leak.

Different approach:

char *
get_ssl_library_version(void)
{
    static char polar_version[30];
    unsigned int pv = version_get_number();
    sprintf( polar_version, "PolarSSL %d.%d.%d",
                (pv>>24)&0xff, (pv>>16)&0xff, (pv>>8)&0xff );
    return polar_version;
}

this is well-defined (polarssl/version.h), and guaranteed to not overflow.

> Or maybe even slightly more hardened.  Put version_get_string_full()
> into a much bigger temporary buffer, which then uses a strncpy() or
> memcpy() into the polar_version[] buffer.  The strncpy()/memcpy() should
> restrict it to 30 bytes, including NULL terminator.

"much bigger", right :-)

gert
-- 
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             g...@greenie.muc.de
fax: +49-89-35655025                        g...@net.informatik.tu-muenchen.de

Attachment: pgpZITQqy8ZSy.pgp
Description: PGP signature

Reply via email to