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 [email protected]
fax: +49-89-35655025 [email protected]
pgpZITQqy8ZSy.pgp
Description: PGP signature
