* [U-Boot] [PATCH v2 2/2] image: Protect against overflow in unknown_msg()
2016-10-27 23:54 [U-Boot] [PATCH v2 1/2] mkimage: Fix missing free() in show_valid_options() Simon Glass
@ 2016-10-27 23:54 ` Simon Glass
0 siblings, 0 replies; 4+ messages in thread
From: Simon Glass @ 2016-10-27 23:54 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH v2 2/2] image: Protect against overflow in unknown_msg()
@ 2016-10-28 0:38 Simon Glass
2016-10-28 1:22 ` Siarhei Siamashka
0 siblings, 1 reply; 4+ messages in thread
From: Simon Glass @ 2016-10-28 0:38 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH v2 2/2] image: Protect against overflow in unknown_msg()
2016-10-28 0:38 [U-Boot] [PATCH v2 2/2] image: Protect against overflow in unknown_msg() Simon Glass
@ 2016-10-28 1:22 ` Siarhei Siamashka
2016-10-28 2:18 ` Simon Glass
0 siblings, 1 reply; 4+ messages in thread
From: Siarhei Siamashka @ 2016-10-28 1:22 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH v2 2/2] image: Protect against overflow in unknown_msg()
2016-10-28 1:22 ` Siarhei Siamashka
@ 2016-10-28 2:18 ` Simon Glass
0 siblings, 0 replies; 4+ messages in thread
From: Simon Glass @ 2016-10-28 2:18 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-10-28 2:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-28 0:38 [U-Boot] [PATCH v2 2/2] image: Protect against overflow in unknown_msg() Simon Glass
2016-10-28 1:22 ` Siarhei Siamashka
2016-10-28 2:18 ` Simon Glass
-- strict thread matches above, loose matches on Subject: below --
2016-10-27 23:54 [U-Boot] [PATCH v2 1/2] mkimage: Fix missing free() in show_valid_options() Simon Glass
2016-10-27 23:54 ` [U-Boot] [PATCH v2 2/2] image: Protect against overflow in unknown_msg() Simon Glass
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox