On 15/04/14 09:42, Gert Doering wrote:> 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.I like this. I know that Paul has promised to never exceed 18 bytes. But you never know what some distros might do. They may modify these strings to provide more fine grained info. But this approach above will actually always be far more predictable. -- kind regards, David Sommerseth
signature.asc
Description: OpenPGP digital signature
