From ef77c453f2de4ee97dd7ae2e9f0ba6c9aafcd874 Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Mon, 13 Feb 2023 20:40:42 +0000 Subject: [image] Check delimiters when parsing command-line key-value arguments The Linux kernel bzImage image format and the CPIO archive constructor will parse the image command line for certain arguments of the form "key=value". This parsing is currently implemented using strstr() in a way that can cause a false positive suffix match. For example, a command line containing "highmem=" would erroneously be treated as containing a value for "mem=". Fix by centralising the logic used for parsing such arguments, and including a check that the argument immediately follows a whitespace delimiter (or is at the start of the string). Signed-off-by: Michael Brown --- src/arch/x86/image/bzimage.c | 37 ++++++++++++++++--------------------- src/core/cpio.c | 9 ++------- src/core/image.c | 31 +++++++++++++++++++++++++++++++ src/include/ipxe/image.h | 1 + 4 files changed, 50 insertions(+), 28 deletions(-) diff --git a/src/arch/x86/image/bzimage.c b/src/arch/x86/image/bzimage.c index 769e0c9..b40bb2d 100644 --- a/src/arch/x86/image/bzimage.c +++ b/src/arch/x86/image/bzimage.c @@ -247,19 +247,17 @@ static void bzimage_update_header ( struct image *image, * * @v image bzImage file * @v bzimg bzImage context - * @v cmdline Kernel command line * @ret rc Return status code */ static int bzimage_parse_cmdline ( struct image *image, - struct bzimage_context *bzimg, - char *cmdline ) { + struct bzimage_context *bzimg ) { + const char *vga; + const char *mem; char *sep; - char *vga; - char *mem; + char *end; /* Look for "vga=" */ - if ( ( vga = strstr ( cmdline, "vga=" ) ) ) { - vga += 4; + if ( ( vga = image_argument ( image, "vga=" ) ) ) { sep = strchr ( vga, ' ' ); if ( sep ) *sep = '\0'; @@ -270,10 +268,10 @@ static int bzimage_parse_cmdline ( struct image *image, } else if ( strcmp ( vga, "ask" ) == 0 ) { bzimg->vid_mode = BZI_VID_MODE_ASK; } else { - bzimg->vid_mode = strtoul ( vga, &vga, 0 ); - if ( *vga ) { + bzimg->vid_mode = strtoul ( vga, &end, 0 ); + if ( *end ) { DBGC ( image, "bzImage %p strange \"vga=\" " - "terminator '%c'\n", image, *vga ); + "terminator '%c'\n", image, *end ); } } if ( sep ) @@ -281,10 +279,9 @@ static int bzimage_parse_cmdline ( struct image *image, } /* Look for "mem=" */ - if ( ( mem = strstr ( cmdline, "mem=" ) ) ) { - mem += 4; - bzimg->mem_limit = strtoul ( mem, &mem, 0 ); - switch ( *mem ) { + if ( ( mem = image_argument ( image, "mem=" ) ) ) { + bzimg->mem_limit = strtoul ( mem, &end, 0 ); + switch ( *end ) { case 'G': case 'g': bzimg->mem_limit <<= 10; @@ -302,7 +299,7 @@ static int bzimage_parse_cmdline ( struct image *image, break; default: DBGC ( image, "bzImage %p strange \"mem=\" " - "terminator '%c'\n", image, *mem ); + "terminator '%c'\n", image, *end ); break; } bzimg->mem_limit -= 1; @@ -316,11 +313,10 @@ static int bzimage_parse_cmdline ( struct image *image, * * @v image bzImage image * @v bzimg bzImage context - * @v cmdline Kernel command line */ static void bzimage_set_cmdline ( struct image *image, - struct bzimage_context *bzimg, - const char *cmdline ) { + struct bzimage_context *bzimg ) { + const char *cmdline = ( image->cmdline ? image->cmdline : "" ); size_t cmdline_len; /* Copy command line down to real-mode portion */ @@ -528,7 +524,6 @@ static void bzimage_load_initrds ( struct image *image, */ static int bzimage_exec ( struct image *image ) { struct bzimage_context bzimg; - char *cmdline = ( image->cmdline ? image->cmdline : "" ); int rc; /* Read and parse header from image */ @@ -551,7 +546,7 @@ static int bzimage_exec ( struct image *image ) { } /* Parse command line for bootloader parameters */ - if ( ( rc = bzimage_parse_cmdline ( image, &bzimg, cmdline ) ) != 0) + if ( ( rc = bzimage_parse_cmdline ( image, &bzimg ) ) != 0) return rc; /* Check that initrds can be loaded */ @@ -568,7 +563,7 @@ static int bzimage_exec ( struct image *image ) { bzimg.rm_filesz, bzimg.pm_sz ); /* Store command line */ - bzimage_set_cmdline ( image, &bzimg, cmdline ); + bzimage_set_cmdline ( image, &bzimg ); /* Prepare for exiting. Must do this before loading initrds, * since loading the initrds will corrupt the external heap. diff --git a/src/core/cpio.c b/src/core/cpio.c index 27aee75..4b607e2 100644 --- a/src/core/cpio.c +++ b/src/core/cpio.c @@ -77,17 +77,12 @@ size_t cpio_name_len ( struct image *image ) { */ static void cpio_parse_cmdline ( struct image *image, struct cpio_header *cpio ) { - const char *cmdline; - char *arg; + const char *arg; char *end; unsigned int mode; - /* Skip image filename */ - cmdline = ( cpio_name ( image ) + cpio_name_len ( image ) ); - /* Look for "mode=" */ - if ( ( arg = strstr ( cmdline, "mode=" ) ) ) { - arg += 5; + if ( ( arg = image_argument ( image, "mode=" ) ) ) { mode = strtoul ( arg, &end, 8 /* Octal for file mode */ ); if ( *end && ( *end != ' ' ) ) { DBGC ( image, "CPIO %p strange \"mode=\" " diff --git a/src/core/image.c b/src/core/image.c index 3e236ca..f6d3d8d 100644 --- a/src/core/image.c +++ b/src/core/image.c @@ -27,6 +27,7 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL ); #include #include #include +#include #include #include #include @@ -569,3 +570,33 @@ struct image * image_memory ( const char *name, userptr_t data, size_t len ) { err_alloc_image: return NULL; } + +/** + * Find argument within image command line + * + * @v image Image + * @v key Argument search key (including trailing delimiter) + * @ret value Argument value, or NULL if not found + */ +const char * image_argument ( struct image *image, const char *key ) { + const char *cmdline = image->cmdline; + const char *search; + const char *match; + const char *next; + + /* Find argument */ + for ( search = cmdline ; search ; search = next ) { + + /* Find next occurrence, if any */ + match = strstr ( search, key ); + if ( ! match ) + break; + next = ( match + strlen ( key ) ); + + /* Check preceding delimiter, if any */ + if ( ( match == cmdline ) || isspace ( match[-1] ) ) + return next; + } + + return NULL; +} diff --git a/src/include/ipxe/image.h b/src/include/ipxe/image.h index 0a5a260..9e0c0f2 100644 --- a/src/include/ipxe/image.h +++ b/src/include/ipxe/image.h @@ -195,6 +195,7 @@ extern struct image * image_find_selected ( void ); extern int image_set_trust ( int require_trusted, int permanent ); extern struct image * image_memory ( const char *name, userptr_t data, size_t len ); +extern const char * image_argument ( struct image *image, const char *key ); extern int image_pixbuf ( struct image *image, struct pixel_buffer **pixbuf ); extern int image_asn1 ( struct image *image, size_t offset, struct asn1_cursor **cursor ); -- cgit v1.1