From 9de64c77f0567ac3fbcc4ce62a7090a6bae84360 Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Fri, 14 Sep 2018 14:35:24 -0600 Subject: Fix various abort(), crashes, and memory errors cbootimage doesn't have extensive error-checking of the input files. Thus it's easy to trigger aborts (which in turn segfault to exit the app) and bad memory accesses by providing under-sized binary input files or configuration files with missing required statements. Add a bit more error-checking to clean up some of these cases. No doubt there are more, but this change only fixes those that have been reported. Signed-off-by: Stephen Warren Reviewed-by: Thierry Reding --- src/cbootimage.c | 15 ++++++++++++++- src/data_layout.c | 11 +++++++++-- src/parse.c | 2 +- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/cbootimage.c b/src/cbootimage.c index ca781fa..e728c8b 100644 --- a/src/cbootimage.c +++ b/src/cbootimage.c @@ -239,7 +239,7 @@ main(int argc, char *argv[]) /* Get BCT_SIZE from input image file */ bct_size = get_bct_size_from_image(&context); - if (bct_size < 0) { + if (bct_size <= 0) { printf("Error: Invalid input image file %s\n", context.input_image_filename); goto fail; @@ -301,6 +301,19 @@ main(int argc, char *argv[]) goto fail; } + if (!context.bct_init) { + e = 1; + printf("No BCT file has been read or generated.\n"); + printf("This is likely due to an incomplete config file.\n"); + goto fail; + } + if (!context.memory) { + e = 1; + printf("No output data generated.\n"); + printf("This is likely due to an incomplete config file.\n"); + goto fail; + } + /* Peform final signing & encryption of bct. */ e = sign_bct(&context, context.bct); if (e != 0) { diff --git a/src/data_layout.c b/src/data_layout.c index 2ed486b..8301aec 100644 --- a/src/data_layout.c +++ b/src/data_layout.c @@ -218,8 +218,10 @@ write_page(build_image_context *context, return -ENOMEM; if (block->data == NULL) return -ENOMEM; - assert(((page_number + 1) * context->page_size) - <= context->block_size); + if (((page_number + 1) * context->page_size) > context->block_size) { + printf("Page number outside block; likely config file error.\n"); + return -ENOMEM; + } if (block->pages_used != page_number) { printf("Warning: Writing page in block out of order.\n"); @@ -838,6 +840,11 @@ begin_update(build_image_context *context) assert(context); + if (context->page_size_log2 < NVBOOT_AES_BLOCK_SIZE_LOG2) { + printf("Page size is too small; likely config file error\n"); + return 1; + } + /* Ensure that the BCT block & page data is current. */ if (enable_debug) { uint32_t block_size_log2; diff --git a/src/parse.c b/src/parse.c index 99cb428..1006093 100644 --- a/src/parse.c +++ b/src/parse.c @@ -249,7 +249,7 @@ parse_filename(char *str, char *name, int chars_remaining) * Check if the filename buffer is out of space, preserving one * character to null terminate the string. */ - while (isalnum(*str) || strchr("\\/~_-+:.@", *str)) { + while (*str && (isalnum(*str) || strchr("\\/~_-+:.@", *str))) { chars_remaining--; -- cgit v1.2.1