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
pgpZITQqy8ZSy.pgp
Description: PGP signature