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

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

[Simon Glass]

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@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);
 
 	return msg;
 }
-- 
2.8.0.rc3.226.g39d4020

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

[Siarhei Siamashka]

On Thu, 27 Oct 2016 18:38:40 -0600
Simon Glass <sjg@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@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;
>  }


-- 
Best regards,
Siarhei Siamashka

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

[Simon Glass]

Hi,

On 27 October 2016 at 18:22, Siarhei Siamashka
<siarhei.siamashka@gmail.com> wrote:
> On Thu, 27 Oct 2016 18:38:40 -0600
> Simon Glass <sjg@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@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

[U-Boot] [PATCH v2 1/2] mkimage: Fix missing free() in show_valid_options()

[Simon Glass]

The allocated memory should be freed. Fix it.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reported-by: Coverity (CID: 150963)
Reviewed-by: Tom Rini <trini@konsulko.com>
---

Changes in v2: None

 tools/mkimage.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/mkimage.c b/tools/mkimage.c
index 3c594a0..521fa80 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -64,6 +64,7 @@ static int show_valid_options(enum ih_category category)
 			genimg_get_cat_name(category, item));
 	}
 	fprintf(stderr, "\n");
+	free(order);
 
 	return 0;
 }
-- 
2.8.0.rc3.226.g39d4020

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

[Simon Glass]

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@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);
 
 	return msg;
 }
-- 
2.8.0.rc3.226.g39d4020