[U-Boot] [PATCH v2 2/2] image: Protect against overflow in unknown_msg()

Simon Glass sjg at chromium.org
Fri Oct 28 04:18:11 CEST 2016


Hi,

On 27 October 2016 at 18:22, Siarhei Siamashka
<siarhei.siamashka at gmail.com> wrote:
> On Thu, 27 Oct 2016 18:38:40 -0600
> Simon Glass <sjg at chromium.org> wrote:
>
>> Coverity complains that this can overflow. If we later increase the size
>> of one of the strings in the table, it could happen.
>>
>> Adjust the code to protect against this.
>>
>> Signed-off-by: Simon Glass <sjg at chromium.org>
>> Reported-by: Coverity (CID: 150964)
>> ---
>>
>> Changes in v2:
>> - Drop unwanted #include
>>
>>  common/image.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/common/image.c b/common/image.c
>> index 0e86c13..4255267 100644
>> --- a/common/image.c
>> +++ b/common/image.c
>> @@ -590,7 +590,7 @@ static const char *unknown_msg(enum ih_category category)
>>       static char msg[30];
>>
>>       strcpy(msg, "Unknown ");
>> -     strcat(msg, table_info[category].desc);
>> +     strncat(msg, table_info[category].desc, sizeof(msg) - 1);
>
> "man strncat" on my system says:
>
>        char *strncat(char *dest, const char *src, size_t n);
>
>        ...
>
>        If src contains n or more bytes, strncat() writes n+1 bytes to
>        dest (n from src plus the terminating null byte).  Therefore,
>        the size of dest must be at least strlen(dest)+n+1.
>
>>
>>       return msg;
>>  }
>

That's odd as it does not seem to match the U-Boot implementation. But
neither does my code in fact. I'll try v3.

I suspect U-Boot's string functions could use some tests!

Regards,
Simon


More information about the U-Boot mailing list