fwts_coreboot_cbmem: fix off-by-one error on last char of buffer
diff mbox series

Message ID 20180814131602.20050-1-colin.king@canonical.com
State Accepted
Headers show
Series
  • fwts_coreboot_cbmem: fix off-by-one error on last char of buffer
Related show

Commit Message

Colin King Aug. 14, 2018, 1:16 p.m. UTC
From: Colin Ian King <colin.king@canonical.com>

The coreboot_log has been allocated a size of console->size + 1,
however, the end of string terminator '\0' is being written one
character passed the end of the buffer because of an off-by-one
error. Fix this.

Also insert some spaces as per the fwts coding style.

Detected by CoverityScan, CID#1394472 ("Out-of-bounds write")

Fixes: c9bf07f25d13 ("fwts_coreboot.c: add cbmem console parser")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/lib/src/fwts_coreboot_cbmem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Alex Hung Aug. 14, 2018, 7:14 p.m. UTC | #1
On 2018-08-14 06:16 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The coreboot_log has been allocated a size of console->size + 1,
> however, the end of string terminator '\0' is being written one
> character passed the end of the buffer because of an off-by-one
> error. Fix this.
> 
> Also insert some spaces as per the fwts coding style.
> 
> Detected by CoverityScan, CID#1394472 ("Out-of-bounds write")
> 
> Fixes: c9bf07f25d13 ("fwts_coreboot.c: add cbmem console parser")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/lib/src/fwts_coreboot_cbmem.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/lib/src/fwts_coreboot_cbmem.c b/src/lib/src/fwts_coreboot_cbmem.c
> index 456ac694..52f83f47 100644
> --- a/src/lib/src/fwts_coreboot_cbmem.c
> +++ b/src/lib/src/fwts_coreboot_cbmem.c
> @@ -334,13 +334,13 @@ char *fwts_coreboot_cbmem_console_dump(void)
>   
>   	free(console_p);
>   
> -	coreboot_log = malloc(console->size+1);
> +	coreboot_log = malloc(console->size + 1);
>   	if (!coreboot_log) {
>   		free(console);
>   		return NULL;
>   	}
>   
> -	coreboot_log[console->size + 1] = '\0';
> +	coreboot_log[console->size] = '\0';
>   
>   	count = memconsole_coreboot_read(console, coreboot_log, 0, console->size);
>   	free(console);
> 

Acked-by: Alex Hung <alex.hung@canonical.com>
ivanhu Aug. 16, 2018, 9:07 a.m. UTC | #2
On 08/14/2018 09:16 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The coreboot_log has been allocated a size of console->size + 1,
> however, the end of string terminator '\0' is being written one
> character passed the end of the buffer because of an off-by-one
> error. Fix this.
>
> Also insert some spaces as per the fwts coding style.
>
> Detected by CoverityScan, CID#1394472 ("Out-of-bounds write")
>
> Fixes: c9bf07f25d13 ("fwts_coreboot.c: add cbmem console parser")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  src/lib/src/fwts_coreboot_cbmem.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/lib/src/fwts_coreboot_cbmem.c b/src/lib/src/fwts_coreboot_cbmem.c
> index 456ac694..52f83f47 100644
> --- a/src/lib/src/fwts_coreboot_cbmem.c
> +++ b/src/lib/src/fwts_coreboot_cbmem.c
> @@ -334,13 +334,13 @@ char *fwts_coreboot_cbmem_console_dump(void)
>  
>  	free(console_p);
>  
> -	coreboot_log = malloc(console->size+1);
> +	coreboot_log = malloc(console->size + 1);
>  	if (!coreboot_log) {
>  		free(console);
>  		return NULL;
>  	}
>  
> -	coreboot_log[console->size + 1] = '\0';
> +	coreboot_log[console->size] = '\0';
>  
>  	count = memconsole_coreboot_read(console, coreboot_log, 0, console->size);
>  	free(console);

Acked-by: Ivan Hu <ivan.hu@canonical.com>

Patch
diff mbox series

diff --git a/src/lib/src/fwts_coreboot_cbmem.c b/src/lib/src/fwts_coreboot_cbmem.c
index 456ac694..52f83f47 100644
--- a/src/lib/src/fwts_coreboot_cbmem.c
+++ b/src/lib/src/fwts_coreboot_cbmem.c
@@ -334,13 +334,13 @@  char *fwts_coreboot_cbmem_console_dump(void)
 
 	free(console_p);
 
-	coreboot_log = malloc(console->size+1);
+	coreboot_log = malloc(console->size + 1);
 	if (!coreboot_log) {
 		free(console);
 		return NULL;
 	}
 
-	coreboot_log[console->size + 1] = '\0';
+	coreboot_log[console->size] = '\0';
 
 	count = memconsole_coreboot_read(console, coreboot_log, 0, console->size);
 	free(console);