From 4f52cda5042f8d1c306ab1de57c4ed8d505b0409 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beat=20K=C3=BCng?= Date: Fri, 26 Aug 2022 14:22:49 +0200 Subject: [PATCH] flashfs: ensure no address past the end of a sector is dereferenced As it might result in a hardfault if it's the last flash sector. --- src/lib/parameters/flashparams/flashfs.c | 32 ++++++++++++++-------- src/lib/parameters/flashparams/flashfs32.c | 27 +++++++++++++----- 2 files changed, 41 insertions(+), 18 deletions(-) diff --git a/src/lib/parameters/flashparams/flashfs.c b/src/lib/parameters/flashparams/flashfs.c index ed1f318b28..b9a8d11801 100644 --- a/src/lib/parameters/flashparams/flashfs.c +++ b/src/lib/parameters/flashparams/flashfs.c @@ -339,7 +339,7 @@ static inline int entry_size_adjust(flash_entry_header_t *fi) static inline flash_entry_header_t *next_entry(flash_entry_header_t *fi) { uint8_t *pb = (uint8_t *)fi; - return (flash_entry_header_t *) &pb[fi->size]; + return (flash_entry_header_t *)(pb + fi->size); } /**************************************************************************** @@ -449,22 +449,29 @@ static flash_entry_header_t *find_entry(flash_file_token_t token) /* Hunt for Magic Signature */ cont: - while (pmagic != pe && !valid_magic(pmagic)) { + while (pmagic < pe && !valid_magic(pmagic)) { pmagic++; } /* Did we reach the end * if so try the next sector */ - if (pmagic == pe) { continue; } + if (pmagic >= pe) { continue; } /* Found a magic So assume it is a file header */ flash_entry_header_t *pf = (flash_entry_header_t *) pmagic; - /* Test the CRC */ + /* Ensure that the header is fully inside the current sector */ - if (pf->crc == crc32(entry_crc_start(pf), entry_crc_length(pf))) { + if (pf + 1 > (flash_entry_header_t *)pe) { continue; } + + const uint8_t *crc_start = entry_crc_start(pf); + data_size_t crc_length = entry_crc_length(pf); + + if (crc_start + crc_length > (uint8_t *)pe) { continue; } + + if (pf->crc == crc32(crc_start, crc_length)) { /* Good CRC is it the one we are looking for ?*/ @@ -481,16 +488,15 @@ cont: /* If the next one is erased */ - if (blank_entry(pf)) { + if (pmagic >= pe || blank_entry(pf)) { continue; } } goto cont; - } else { - /* in valid CRC so keep looking */ + /* invalid CRC so keep looking */ pmagic++; } @@ -529,6 +535,10 @@ static flash_entry_header_t *find_free(data_size_t required) flash_entry_header_t *pf = (flash_entry_header_t *) pmagic; + /* Ensure that the header is fully inside the current sector */ + + if (pf + 1 > (flash_entry_header_t *)pe) { break; } + /* Test the CRC */ if (pf->crc == crc32(entry_crc_start(pf), entry_crc_length(pf))) { @@ -543,7 +553,7 @@ static flash_entry_header_t *find_free(data_size_t required) } } - if (blank_magic(pmagic)) { + if (pmagic + (required / sizeof(h_magic_t)) <= pe && blank_magic(pmagic)) { flash_entry_header_t *pf = (flash_entry_header_t *) pmagic; @@ -552,7 +562,7 @@ static flash_entry_header_t *find_free(data_size_t required) } } - } while (++pmagic != pe); + } while (++pmagic < pe); } return NULL; @@ -595,7 +605,7 @@ static sector_descriptor_t *get_next_sector_descriptor(sector_descriptor_t * } /**************************************************************************** - * Name: get_next_sector + * Name: get_sector_info * * Description: * Given a pointer to a flash entry header returns the sector descriptor diff --git a/src/lib/parameters/flashparams/flashfs32.c b/src/lib/parameters/flashparams/flashfs32.c index 662d4bb1f1..368c011a14 100644 --- a/src/lib/parameters/flashparams/flashfs32.c +++ b/src/lib/parameters/flashparams/flashfs32.c @@ -348,7 +348,7 @@ static inline int entry_size_adjust(flash_entry_header_t *fi) static inline flash_entry_header_t *next_entry(flash_entry_header_t *fi) { uint8_t *pb = (uint8_t *)fi; - return (flash_entry_header_t *) &pb[fi->size]; + return (flash_entry_header_t *)(pb + fi->size); } /**************************************************************************** @@ -458,20 +458,29 @@ static flash_entry_header_t *find_entry(flash_file_token_t token) /* Hunt for Magic Signature */ cont: - while (pmagic != pe && !valid_magic(pmagic)) { + while (pmagic < pe && !valid_magic(pmagic)) { pmagic++; } /* Did we reach the end * if so try the next sector */ - if (pmagic == pe) { continue; } + if (pmagic >= pe) { continue; } /* Found a magic So assume it is a file header */ flash_entry_header_t *pf = (flash_entry_header_t *) pmagic; - if (pf->crc == crc32(entry_crc_start(pf), entry_crc_length(pf))) { + /* Ensure that the header is fully inside the current sector */ + + if (pf + 1 > (flash_entry_header_t *)pe) { continue; } + + const uint8_t *crc_start = entry_crc_start(pf); + data_size_t crc_length = entry_crc_length(pf); + + if (crc_start + crc_length > (uint8_t *)pe) { continue; } + + if (pf->crc == crc32(crc_start, crc_length)) { /* Good CRC is it the one we are looking for ?*/ @@ -488,7 +497,7 @@ cont: /* If the next one is erased */ - if (blank_entry(pf)) { + if (pmagic >= pe || blank_entry(pf)) { continue; } } @@ -536,6 +545,10 @@ static flash_entry_header_t *find_free(data_size_t required) flash_entry_header_t *pf = (flash_entry_header_t *) pmagic; + /* Ensure that the header is fully inside the current sector */ + + if (pf + 1 > (flash_entry_header_t *)pe) { break; } + /* Test the CRC */ if (pf->crc == crc32(entry_crc_start(pf), entry_crc_length(pf))) { @@ -550,7 +563,7 @@ static flash_entry_header_t *find_free(data_size_t required) } } - if (blank_magic(pmagic)) { + if (pmagic + (required / sizeof(h_magic_t)) <= pe && blank_magic(pmagic)) { flash_entry_header_t *pf = (flash_entry_header_t *) pmagic; @@ -559,7 +572,7 @@ static flash_entry_header_t *find_free(data_size_t required) } } - } while (++pmagic != pe); + } while (++pmagic < pe); } return NULL;