From e495aebb35613e05251df4967b8230592414b961 Mon Sep 17 00:00:00 2001 From: Davide Bettio Date: Mon, 22 Jun 2026 10:15:12 +0000 Subject: [PATCH 1/6] Fix out-of-bounds read on avmpack lookup miss The packbeam scanners were unbounded, so a lookup miss (e.g. read_priv for an unpacked file) ran off the pack into erased flash and crashed. Bound and validate every scan against the stored pack size. The walk now stops only at the end marker: the old scanners stopped at the first data file (flags 0), hiding every section after it. Signed-off-by: Davide Bettio --- CHANGELOG.md | 2 + src/libAtomVM/avmpack.c | 156 ++++++++++++------- src/libAtomVM/avmpack.h | 42 +++-- src/libAtomVM/globalcontext.c | 3 +- src/libAtomVM/jit_stream_flash.c | 28 +++- src/libAtomVM/nifs.c | 45 ++++-- src/platforms/emscripten/src/lib/sys.c | 5 +- src/platforms/emscripten/src/main.c | 3 +- src/platforms/esp32/components/avm_sys/sys.c | 4 +- src/platforms/esp32/main/main.c | 5 +- src/platforms/esp32/test/main/test_main.c | 4 +- src/platforms/generic_unix/lib/sys.c | 2 +- src/platforms/generic_unix/main.c | 6 +- src/platforms/rp2/src/main.c | 10 +- src/platforms/rp2/tests/test_main.c | 3 +- src/platforms/stm32/src/main.c | 6 +- tests/test-jit_stream_flash.c | 66 +++++++- 17 files changed, 285 insertions(+), 105 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 730448da5c..f7938d8557 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,6 +86,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed a JIT crash (`EXC_BAD_ACCESS`/SIGBUS) on Apple Silicon - Fixed the ESP32 event poller re-blocking after running a listener, which could delay a process readied by a driver (e.g. an active-mode socket message) until the next event or timer tick +- Fixed AVM pack lookups not finding sections placed after a data file +- Fixed an out-of-bounds read crashing the VM on an AVM pack section lookup miss (e.g. `atomvm:read_priv/2` for an unpacked file) ## [0.7.0-alpha.1] - 2026-04-06 diff --git a/src/libAtomVM/avmpack.c b/src/libAtomVM/avmpack.c index 624fa82493..19cdf6a121 100644 --- a/src/libAtomVM/avmpack.c +++ b/src/libAtomVM/avmpack.c @@ -27,8 +27,11 @@ #include #define AVMPACK_SIZE 24 +#define AVMPACK_SECTION_HEADER_SIZE 12 +#define AVMPACK_MIN_SECTION_SIZE 16 +#define AVMPACK_END_MARKER_SIZE 16 -static inline int pad(int size) +static inline size_t pad(size_t size) { return ((size + 4 - 1) >> 2) << 2; } @@ -52,79 +55,126 @@ bool avmpack_is_valid(const void *avmpack_binary, uint32_t size) return memcmp(avmpack_binary, pack_header, AVMPACK_SIZE) == 0; } -int avmpack_find_section_by_flag(const void *avmpack_binary, uint32_t flags_mask, uint32_t flags_val, const void **ptr, uint32_t *size, const char **name) +enum AVMPackSectionKind { - int offset = AVMPACK_SIZE; - const uint32_t *flags; + AVMPackSectionInvalid, + AVMPackSectionRegular, + AVMPackSectionEnd +}; - do { - const uint32_t *sizes = ((const uint32_t *) (avmpack_binary)) + offset / sizeof(uint32_t); - flags = ((const uint32_t *) (avmpack_binary)) + 1 + offset / sizeof(uint32_t); +struct AVMPackSection +{ + uint32_t size; + uint32_t data_size; + uint32_t flags; + const char *name; + const void *data; +}; - if ((ENDIAN_SWAP_32(*flags) & flags_mask) == flags_val) { - const char *found_section_name = (const char *) (sizes + 3); - int section_name_len = pad(strlen(found_section_name) + 1); +static enum AVMPackSectionKind read_section(const void *avmpack_binary, uint32_t avmpack_size, + uint32_t offset, struct AVMPackSection *section) +{ + if (offset > avmpack_size || avmpack_size - offset < AVMPACK_MIN_SECTION_SIZE) { + return AVMPackSectionInvalid; + } - *ptr = sizes + 3 + section_name_len / sizeof(uint32_t); - *size = ENDIAN_SWAP_32(*sizes); - *name = (const char *) (sizes + 3); - return 1; + // A pack may sit at any byte offset (e.g. atomvm:add_avm_pack_binary/2 with a sub-binary). + const uint8_t *header = (const uint8_t *) avmpack_binary + offset; + uint32_t section_size = READ_32_UNALIGNED(header); + uint32_t flags = READ_32_UNALIGNED(header + 4); + const char *name = (const char *) (header + AVMPACK_SECTION_HEADER_SIZE); + + if (section_size == 0) { + // The min-size guard above keeps the terminator name bytes in bounds for strcmp. + if (flags == 0 && (strcmp(name, "end") == 0 || strcmp(name, "END") == 0)) { + section->size = 0; + section->data_size = 0; + section->flags = 0; + section->name = name; + section->data = (const uint8_t *) avmpack_binary + offset + AVMPACK_END_MARKER_SIZE; + return AVMPackSectionEnd; } + return AVMPackSectionInvalid; + } - offset += ENDIAN_SWAP_32(*sizes); + if (section_size < AVMPACK_MIN_SECTION_SIZE || (section_size & 3) != 0 + || section_size > avmpack_size - offset) { + return AVMPackSectionInvalid; + } + + size_t name_region = section_size - AVMPACK_SECTION_HEADER_SIZE; + const void *nul = memchr(name, '\0', name_region); + if (nul == NULL) { + return AVMPackSectionInvalid; + } + size_t padded_name_len = pad((size_t) ((const char *) nul - name) + 1); + if (padded_name_len > name_region) { + return AVMPackSectionInvalid; + } - } while (*flags); + section->size = section_size; + section->data_size = section_size - AVMPACK_SECTION_HEADER_SIZE - (uint32_t) padded_name_len; + section->flags = flags; + section->name = name; + section->data + = (const uint8_t *) avmpack_binary + offset + AVMPACK_SECTION_HEADER_SIZE + padded_name_len; - return 0; + return AVMPackSectionRegular; } -int avmpack_find_section_by_name(const void *avmpack_binary, const char *name, const void **ptr, uint32_t *size) +int avmpack_find_section_by_flag(const void *avmpack_binary, uint32_t avmpack_size, + uint32_t flags_mask, uint32_t flags_val, const void **ptr, uint32_t *size, const char **name) { - int offset = AVMPACK_SIZE; - const uint32_t *flags; + uint32_t offset = AVMPACK_SIZE; + struct AVMPackSection section; + enum AVMPackSectionKind kind; + + while ((kind = read_section(avmpack_binary, avmpack_size, offset, §ion)) + != AVMPackSectionInvalid) { + if ((section.flags & flags_mask) == flags_val) { + *ptr = section.data; + *size = section.data_size; + *name = section.name; + return 1; + } + if (kind == AVMPackSectionEnd) { + break; + } + offset += section.size; + } - do { - const uint32_t *sizes = ((const uint32_t *) (avmpack_binary)) + offset / sizeof(uint32_t); - flags = ((const uint32_t *) (avmpack_binary)) + 1 + offset / sizeof(uint32_t); + return 0; +} - const char *found_section_name = (const char *) (sizes + 3); - if (!strcmp(name, found_section_name)) { - int section_name_len = pad(strlen(found_section_name) + 1); +int avmpack_find_section_by_name(const void *avmpack_binary, uint32_t avmpack_size, + const char *name, const void **ptr, uint32_t *size) +{ + uint32_t offset = AVMPACK_SIZE; + struct AVMPackSection section; - *ptr = sizes + 3 + section_name_len / sizeof(uint32_t); - *size = ENDIAN_SWAP_32(*sizes); + while (read_section(avmpack_binary, avmpack_size, offset, §ion) == AVMPackSectionRegular) { + if (!strcmp(name, section.name)) { + *ptr = section.data; + *size = section.data_size; return 1; } - - offset += ENDIAN_SWAP_32(*sizes); - - } while (*flags); + offset += section.size; + } return 0; } -void *avmpack_fold(void *accum, const void *avmpack_binary, avmpack_fold_fun fold_fun) +void *avmpack_fold( + void *accum, const void *avmpack_binary, uint32_t avmpack_size, avmpack_fold_fun fold_fun) { - int offset = AVMPACK_SIZE; - uint32_t size = 0; - - do { - const uint32_t *size_ptr = ((const uint32_t *) (avmpack_binary)) + offset / sizeof(uint32_t); - size = ENDIAN_SWAP_32(*size_ptr); - if (size > 0) { - const uint32_t *flags_ptr = size_ptr + 1; - uint32_t flags = ENDIAN_SWAP_32(*flags_ptr); - const char *section_name = (const char *) (size_ptr + 3); - int section_name_len = pad(strlen(section_name) + 1); - accum = fold_fun( - accum, - size_ptr, size, - size_ptr + 3 + section_name_len / sizeof(uint32_t), - flags, - section_name); - offset += size; - } - } while (size > 0); + uint32_t offset = AVMPACK_SIZE; + struct AVMPackSection section; + + while (read_section(avmpack_binary, avmpack_size, offset, §ion) == AVMPackSectionRegular) { + accum = fold_fun(accum, (const uint8_t *) avmpack_binary + offset, section.size, + section.data, section.flags, section.name); + offset += section.size; + } return accum; } diff --git a/src/libAtomVM/avmpack.h b/src/libAtomVM/avmpack.h index a68e6e3378..26481ef930 100644 --- a/src/libAtomVM/avmpack.h +++ b/src/libAtomVM/avmpack.h @@ -55,14 +55,17 @@ struct AVMPackData bool in_use; int name_atom_id; const void *data; + uint32_t size; }; -static inline void avmpack_data_init(struct AVMPackData *avm_pack_data, const struct AVMPackInfo *info) +static inline void avmpack_data_init( + struct AVMPackData *avm_pack_data, const struct AVMPackInfo *info, uint32_t size) { avm_pack_data->obj_info = info; avm_pack_data->in_use = false; avm_pack_data->name_atom_id = 0; avm_pack_data->data = NULL; + avm_pack_data->size = size; } static inline void avmpack_data_destroy(struct AVMPackData *avm_pack_data, GlobalContext *global) @@ -102,34 +105,48 @@ typedef void *(*avmpack_fold_fun)(void *accum, const void *section_ptr, uint32_t /** * @brief Finds an AVM Pack section that has certain flags set. * - * @details Finds an AVM Pack section that has certain flags set and returns a pointer to it, its size and its name. + * @details Finds an AVM Pack section that has certain flags set and returns a pointer to it, its + * size and its name. The scan is bounded by \p avmpack_size and validates every section header, so + * a miss (or a truncated/corrupt pack) returns 0 instead of reading past the end of the pack. * @param avmpack_binary a pointer to valid AVM Pack file data. + * @param avmpack_size the size in bytes of the AVM Pack (used to bound the scan). * @param flags_mask that will be matched against file sections. * @param flags_value that will be matched against file sections. * @param ptr will point to the found file section. - * @param size will be set to the file section size that has been found, if the section has not been found it will not be updated. + * @param size will be set to the number of bytes readable at \p ptr (the section payload, which + * excludes the section header and the padded name), if the section has not been found it will not + * be updated. * @param name the section name, as defined in the module header. * @returns 1 if the file section has been found, 0 otherwise. */ -int avmpack_find_section_by_flag(const void *avmpack_binary, uint32_t flags_mask, uint32_t flags_value, const void **ptr, uint32_t *size, const char **name); +int avmpack_find_section_by_flag(const void *avmpack_binary, uint32_t avmpack_size, + uint32_t flags_mask, uint32_t flags_value, const void **ptr, uint32_t *size, const char **name); /** * @brief Finds an AVM Pack section that has certain name. * * @details Finds an AVM Pack section with a certain name and returns a pointer to it and its size. + * The scan is bounded by \p avmpack_size and validates every section header, so a miss (or a + * truncated/corrupt pack) returns 0 instead of reading past the end of the pack. * @param avmpack_binary a pointer to valid AVM Pack file data. + * @param avmpack_size the size in bytes of the AVM Pack (used to bound the scan). * @param name the file section name that will be searched. - * @param ptr will point to the found file section, if the section has not been found it will not be updated. - * @param size will be set to the file section size that has been found, if the section has not been found it will not be updated. + * @param ptr will point to the found file section, if the section has not been found it will not be + * updated. + * @param size will be set to the number of bytes readable at \p ptr (the section payload, which + * excludes the section header and the padded name), if the section has not been found it will not + * be updated. * @returns 1 if the file section has been found, 0 otherwise. */ -int avmpack_find_section_by_name(const void *avmpack_binary, const char *name, const void **ptr, uint32_t *size); +int avmpack_find_section_by_name(const void *avmpack_binary, uint32_t avmpack_size, + const char *name, const void **ptr, uint32_t *size); /** - * @brief Returns \c true if the pointed binary is a valid AVM Pack. + * @brief Returns \c true if the pointed binary starts with a valid AVM Pack header. * - * @details Returns if the pointed binary is a valid AVM Pack binary or not. + * @details Performs a cheap check of the 24-byte magic header only. It does not validate the + * section chain or detect truncation. * @param avmpack_binary a pointer to an AVM Pack binary. * @param size the size of AVM Pack binary. * @returns \c true if it is a valid AVM Pack binary, \c false otherwise. @@ -140,12 +157,15 @@ bool avmpack_is_valid(const void *avmpack_binary, uint32_t size); * @brief Fold over all the sections in an AVM Pack. * * @details This function will call the callback on each section of the AVM Pack, passing in - * the current section of each module in the AVM binary to the supplied fold function. + * the current section of each module in the AVM binary to the supplied fold function. The scan + * is bounded by \p avmpack_size and stops on the terminator or on the first invalid section header. * @param accum The accumulator supplied by the application. * @param avmpack_binary a pointer to an AVM Pack binary. + * @param avmpack_size the size in bytes of the AVM Pack (used to bound the scan). * @param fold_fun function that will be called for each AVM section. */ -void *avmpack_fold(void *accum, const void *avmpack_binary, avmpack_fold_fun fold_fun); +void *avmpack_fold( + void *accum, const void *avmpack_binary, uint32_t avmpack_size, avmpack_fold_fun fold_fun); #ifdef __cplusplus } diff --git a/src/libAtomVM/globalcontext.c b/src/libAtomVM/globalcontext.c index cc85e4620b..e76b383339 100644 --- a/src/libAtomVM/globalcontext.c +++ b/src/libAtomVM/globalcontext.c @@ -727,7 +727,8 @@ Module *globalcontext_load_module_from_avm(GlobalContext *global, const char *mo LIST_FOR_EACH (item, avmpack_data) { struct AVMPackData *avmpack_data = GET_LIST_ENTRY(item, struct AVMPackData, avmpack_head); avmpack_data->in_use = true; - if (avmpack_find_section_by_name(avmpack_data->data, module_name, &beam_module, &beam_module_size)) { + if (avmpack_find_section_by_name(avmpack_data->data, avmpack_data->size, module_name, + &beam_module, &beam_module_size)) { break; } } diff --git a/src/libAtomVM/jit_stream_flash.c b/src/libAtomVM/jit_stream_flash.c index d53245dca8..40e85f713b 100644 --- a/src/libAtomVM/jit_stream_flash.c +++ b/src/libAtomVM/jit_stream_flash.c @@ -145,7 +145,11 @@ static struct JITEntry *globalcontext_find_first_jit_entry(GlobalContext *global struct ListHead *avmpack_data = synclist_rdlock(&global->avmpack_data); LIST_FOR_EACH (item, avmpack_data) { struct AVMPackData *avmpack_data = GET_LIST_ENTRY(item, struct AVMPackData, avmpack_head); - avmpack_find_section_by_flag(avmpack_data->data, END_OF_FILE_MASK, END_OF_FILE, &end_offset, &end_size, &end_name); + if (!avmpack_find_section_by_flag(avmpack_data->data, avmpack_data->size, END_OF_FILE_MASK, + END_OF_FILE, &end_offset, &end_size, &end_name)) { + valid_cache = false; + continue; + } valid_cache = valid_cache && (strcmp(end_name, "END") == 0); if (end_offset > max_end_offset) { @@ -154,6 +158,13 @@ static struct JITEntry *globalcontext_find_first_jit_entry(GlobalContext *global } synclist_unlock(&global->avmpack_data); + if (IS_NULL_PTR(max_end_offset)) { + // No pack has a terminator: there is no known place to append, and the arithmetic below + // would wrap to 0. + *is_valid = false; + return NULL; + } + uintptr_t max_end_offset_page = ((((uintptr_t) max_end_offset) - 1) & ~(FLASH_SECTOR_SIZE - 1)); *is_valid = valid_cache; @@ -180,17 +191,26 @@ static void globalcontext_set_cache_valid(GlobalContext *global) do { valid_cache = true; + bool found_end = true; struct ListHead *item; struct ListHead *avmpack_data = synclist_rdlock(&global->avmpack_data); LIST_FOR_EACH (item, avmpack_data) { struct AVMPackData *avmpack_data = GET_LIST_ENTRY(item, struct AVMPackData, avmpack_head); - avmpack_find_section_by_flag(avmpack_data->data, END_OF_FILE_MASK, END_OF_FILE, &end_offset, &end_size, &end_name); + if (!avmpack_find_section_by_flag(avmpack_data->data, avmpack_data->size, + END_OF_FILE_MASK, END_OF_FILE, &end_offset, &end_size, &end_name)) { + found_end = false; + valid_cache = false; + break; + } if (strcmp(end_name, "END")) { valid_cache = false; break; } } synclist_unlock(&global->avmpack_data); + if (!found_end) { + break; + } if (!valid_cache) { // Replace "end" with "END" - this is a 3-byte string replacement const uint8_t end_str[] = "END"; @@ -398,6 +418,10 @@ static term nif_jit_stream_flash_new(Context *ctx, int argc, term argv[]) // No valid entries, get the first position bool is_valid; new_entry = globalcontext_find_first_jit_entry(ctx->global, &is_valid); + if (IS_NULL_PTR(new_entry)) { + fprintf(stderr, "No valid position to append JIT code to\n"); + RAISE_ERROR(BADARG_ATOM); + } } else { // Get position after last valid entry new_entry = jit_entry_next(last_valid_entry); diff --git a/src/libAtomVM/nifs.c b/src/libAtomVM/nifs.c index 0cc23fbfbd..819eb339bf 100644 --- a/src/libAtomVM/nifs.c +++ b/src/libAtomVM/nifs.c @@ -5573,7 +5573,7 @@ static term nif_atomvm_add_avm_pack_binary(Context *ctx, int argc, term argv[]) size_t bin_size = term_binary_size(binary); - if (UNLIKELY(!avmpack_is_valid(term_binary_data(binary), bin_size))) { + if (UNLIKELY(bin_size > UINT32_MAX || !avmpack_is_valid(term_binary_data(binary), bin_size))) { RAISE_ERROR(BADARG_ATOM); } @@ -5589,7 +5589,7 @@ static term nif_atomvm_add_avm_pack_binary(Context *ctx, int argc, term argv[]) if (IS_NULL_PTR(refc_bin_avm)) { RAISE_ERROR(OUT_OF_MEMORY_ATOM); } - avmpack_data_init(&refc_bin_avm->base, &refc_binary_avm_pack_info); + avmpack_data_init(&refc_bin_avm->base, &refc_binary_avm_pack_info, (uint32_t) bin_size); refc_bin_avm->base.data = (const uint8_t *) term_binary_data(binary); struct RefcBinary *refc_bin = (struct RefcBinary *) term_refc_binary_ptr(binary); refc_binary_increment_refcount(refc_bin); @@ -5602,7 +5602,7 @@ static term nif_atomvm_add_avm_pack_binary(Context *ctx, int argc, term argv[]) if (IS_NULL_PTR(const_avm)) { RAISE_ERROR(OUT_OF_MEMORY_ATOM); } - avmpack_data_init(&const_avm->base, &const_avm_pack_info); + avmpack_data_init(&const_avm->base, &const_avm_pack_info, (uint32_t) bin_size); const_avm->base.data = (const uint8_t *) term_binary_data(binary); avmpack_data = &const_avm->base; @@ -5620,7 +5620,7 @@ static term nif_atomvm_add_avm_pack_binary(Context *ctx, int argc, term argv[]) free(allocated_data); RAISE_ERROR(OUT_OF_MEMORY_ATOM); } - avmpack_data_init(&in_memory_avm->base, &in_memory_avm_pack_info); + avmpack_data_init(&in_memory_avm->base, &in_memory_avm_pack_info, (uint32_t) bin_size); in_memory_avm->base.data = (const uint8_t *) allocated_data; avmpack_data = &in_memory_avm->base; @@ -5760,7 +5760,8 @@ static term nif_atomvm_get_start_beam(Context *ctx, int argc, term argv[]) uint32_t size; const void *beam; const char *module_name; - if (!avmpack_find_section_by_flag(avmpack_data->data, BEAM_START_FLAG, BEAM_START_FLAG, &beam, &size, &module_name)) { + if (!avmpack_find_section_by_flag(avmpack_data->data, avmpack_data->size, + BEAM_START_FLAG, BEAM_START_FLAG, &beam, &size, &module_name)) { synclist_unlock(&ctx->global->avmpack_data); if (UNLIKELY(memory_ensure_free(ctx, TUPLE_SIZE(2)) != MEMORY_GC_OK)) { RAISE_ERROR(OUT_OF_MEMORY_ATOM); @@ -5840,17 +5841,27 @@ static term nif_atomvm_read_priv(Context *ctx, int argc, term argv[]) struct AVMPackData *avmpack_data = GET_LIST_ENTRY(item, struct AVMPackData, avmpack_head); bool prev_in_use = avmpack_data->in_use; avmpack_data->in_use = true; - if (avmpack_find_section_by_name(avmpack_data->data, complete_path, &bin_data, &size)) { - uint32_t file_size = READ_32_ALIGNED((uint32_t *) bin_data); - free(complete_path); - complete_path = NULL; - if (UNLIKELY(memory_ensure_free_opt(ctx, TERM_BOXED_REFC_BINARY_SIZE, MEMORY_CAN_SHRINK) != MEMORY_GC_OK)) { - avmpack_data->in_use = prev_in_use; - synclist_unlock(&glb->avmpack_data); - RAISE_ERROR(OUT_OF_MEMORY_ATOM); + if (avmpack_find_section_by_name( + avmpack_data->data, avmpack_data->size, complete_path, &bin_data, &size)) { + // Bound the length prefix to the section payload against a corrupt file size. + if (size >= sizeof(uint32_t)) { + uint32_t file_size = READ_32_ALIGNED((uint32_t *) bin_data); + if (file_size <= size - sizeof(uint32_t)) { + free(complete_path); + complete_path = NULL; + if (UNLIKELY(memory_ensure_free_opt( + ctx, TERM_BOXED_REFC_BINARY_SIZE, MEMORY_CAN_SHRINK) + != MEMORY_GC_OK)) { + avmpack_data->in_use = prev_in_use; + synclist_unlock(&glb->avmpack_data); + RAISE_ERROR(OUT_OF_MEMORY_ATOM); + } + result = term_from_const_binary(((uint8_t *) bin_data) + sizeof(uint32_t), + file_size, &ctx->heap, ctx->global); + break; + } } - result = term_from_const_binary(((uint8_t *) bin_data) + sizeof(uint32_t), file_size, &ctx->heap, ctx->global); - break; + avmpack_data->in_use = prev_in_use; } else { avmpack_data->in_use = prev_in_use; } @@ -6398,7 +6409,7 @@ static term nif_code_all_available(Context *ctx, int argc, term argv[]) LIST_FOR_EACH (item, avmpack_data) { struct AVMPackData *avmpack_data = GET_LIST_ENTRY(item, struct AVMPackData, avmpack_head); acc.avmpack_data = avmpack_data; - avmpack_fold(&acc, avmpack_data->data, nif_code_all_available_fold); + avmpack_fold(&acc, avmpack_data->data, avmpack_data->size, nif_code_all_available_fold); } size_t available_count = acc.acc_count + ctx->global->loaded_modules_count; @@ -6414,7 +6425,7 @@ static term nif_code_all_available(Context *ctx, int argc, term argv[]) LIST_FOR_EACH (item, avmpack_data) { struct AVMPackData *avmpack_data = GET_LIST_ENTRY(item, struct AVMPackData, avmpack_head); acc.avmpack_data = avmpack_data; - avmpack_fold(&acc, avmpack_data->data, nif_code_all_available_fold); + avmpack_fold(&acc, avmpack_data->data, avmpack_data->size, nif_code_all_available_fold); } synclist_unlock(&ctx->global->avmpack_data); diff --git a/src/platforms/emscripten/src/lib/sys.c b/src/platforms/emscripten/src/lib/sys.c index 1a234c9cee..8615e0403c 100644 --- a/src/platforms/emscripten/src/lib/sys.c +++ b/src/platforms/emscripten/src/lib/sys.c @@ -690,7 +690,8 @@ enum OpenAVMResult sys_open_avm_from_file( UNUSED(global); emscripten_fetch_t *fetch = NULL; - void *data = load_or_fetch_file(path, &fetch, NULL); + size_t data_size = 0; + void *data = load_or_fetch_file(path, &fetch, &data_size); if (IS_NULL_PTR(data)) { return AVM_OPEN_CANNOT_OPEN; } @@ -704,7 +705,7 @@ enum OpenAVMResult sys_open_avm_from_file( } return AVM_OPEN_FAILED_ALLOC; } - avmpack_data_init(&const_avm->base, &const_avm_pack_info); + avmpack_data_init(&const_avm->base, &const_avm_pack_info, (uint32_t) data_size); const_avm->base.data = (const uint8_t *) data; *avm_data = &const_avm->base; diff --git a/src/platforms/emscripten/src/main.c b/src/platforms/emscripten/src/main.c index 756bfb43fa..7682a69dc6 100644 --- a/src/platforms/emscripten/src/main.c +++ b/src/platforms/emscripten/src/main.c @@ -59,7 +59,8 @@ static int load_module(const char *path) const void *startup_beam = NULL; uint32_t startup_beam_size; const char *startup_module_name; - avmpack_find_section_by_flag(avmpack_data->data, BEAM_START_FLAG, BEAM_START_FLAG, &startup_beam, &startup_beam_size, &startup_module_name); + avmpack_find_section_by_flag(avmpack_data->data, avmpack_data->size, BEAM_START_FLAG, + BEAM_START_FLAG, &startup_beam, &startup_beam_size, &startup_module_name); if (startup_beam) { avmpack_data->in_use = true; main_module = module_new_from_iff_binary(global, startup_beam, startup_beam_size); diff --git a/src/platforms/esp32/components/avm_sys/sys.c b/src/platforms/esp32/components/avm_sys/sys.c index 10909d9750..9ece8a26fd 100644 --- a/src/platforms/esp32/components/avm_sys/sys.c +++ b/src/platforms/esp32/components/avm_sys/sys.c @@ -458,7 +458,7 @@ enum OpenAVMResult sys_open_avm_from_file(GlobalContext *global, const char *pat if (IS_NULL_PTR(part_avm)) { return AVM_OPEN_FAILED_ALLOC; } - avmpack_data_init(&part_avm->base, &esp32_part_avm_pack_info); + avmpack_data_init(&part_avm->base, &esp32_part_avm_pack_info, size); part_avm->base.data = part_data; part_avm->part_handle = part_handle; avmpack_data = &part_avm->base; @@ -500,7 +500,7 @@ enum OpenAVMResult sys_open_avm_from_file(GlobalContext *global, const char *pat free(file_data); return AVM_OPEN_FAILED_ALLOC; } - avmpack_data_init(&in_memory_avm->base, &in_memory_avm_pack_info); + avmpack_data_init(&in_memory_avm->base, &in_memory_avm_pack_info, size); in_memory_avm->base.data = file_data; avmpack_data = &in_memory_avm->base; } diff --git a/src/platforms/esp32/main/main.c b/src/platforms/esp32/main/main.c index a91a3755de..8ee2e3c199 100644 --- a/src/platforms/esp32/main/main.c +++ b/src/platforms/esp32/main/main.c @@ -99,7 +99,8 @@ void app_main() ESP_LOGE(TAG, "Invalid startup avmpack. size=%u", size); AVM_ABORT(); } - if (!avmpack_find_section_by_flag(startup_avm, BEAM_START_FLAG, BEAM_START_FLAG, &startup_beam, &startup_beam_size, &startup_module_name)) { + if (!avmpack_find_section_by_flag(startup_avm, size, BEAM_START_FLAG, BEAM_START_FLAG, + &startup_beam, &startup_beam_size, &startup_module_name)) { ESP_LOGE(TAG, "Error: Failed to locate start module in startup partition. (Did you flash a library by mistake?)"); AVM_ABORT(); } @@ -109,7 +110,7 @@ void app_main() ESP_LOGE(TAG, "Memory error: Cannot allocate AVMPackData for main.avm."); AVM_ABORT(); } - avmpack_data_init(&avmpack_data->base, &const_avm_pack_info); + avmpack_data_init(&avmpack_data->base, &const_avm_pack_info, size); avmpack_data->base.in_use = true; avmpack_data->base.data = startup_avm; synclist_append(&glb->avmpack_data, &avmpack_data->base.avmpack_head); diff --git a/src/platforms/esp32/test/main/test_main.c b/src/platforms/esp32/test/main/test_main.c index 480b164695..4aff54ce77 100644 --- a/src/platforms/esp32/test/main/test_main.c +++ b/src/platforms/esp32/test/main/test_main.c @@ -147,7 +147,7 @@ term avm_test_case(const char *test_module) struct ConstAVMPack *avmpack_data = malloc(sizeof(struct ConstAVMPack)); TEST_ASSERT(avmpack_data != NULL); - avmpack_data_init(&avmpack_data->base, &const_avm_pack_info); + avmpack_data_init(&avmpack_data->base, &const_avm_pack_info, size); avmpack_data->base.in_use = true; avmpack_data->base.data = main_avm; synclist_append(&glb->avmpack_data, &avmpack_data->base.avmpack_head); @@ -195,7 +195,7 @@ TEST_CASE("test_jit_compile", "[test_run]") struct ConstAVMPack *avmpack_data = malloc(sizeof(struct ConstAVMPack)); TEST_ASSERT(avmpack_data != NULL); - avmpack_data_init(&avmpack_data->base, &const_avm_pack_info); + avmpack_data_init(&avmpack_data->base, &const_avm_pack_info, size); avmpack_data->base.in_use = true; avmpack_data->base.data = main_avm; synclist_append(&glb->avmpack_data, &avmpack_data->base.avmpack_head); diff --git a/src/platforms/generic_unix/lib/sys.c b/src/platforms/generic_unix/lib/sys.c index ce6a032fc9..5145b756dd 100644 --- a/src/platforms/generic_unix/lib/sys.c +++ b/src/platforms/generic_unix/lib/sys.c @@ -444,7 +444,7 @@ enum OpenAVMResult sys_open_avm_from_file( mapped_file_close(mapped); return AVM_OPEN_FAILED_ALLOC; } - avmpack_data_init(&avmpack_data->base, &mapped_file_avm_pack_info); + avmpack_data_init(&avmpack_data->base, &mapped_file_avm_pack_info, (uint32_t) mapped->size); avmpack_data->base.data = mapped->mapped; avmpack_data->mapped = mapped; diff --git a/src/platforms/generic_unix/main.c b/src/platforms/generic_unix/main.c index 3ede61e821..dce88a673c 100644 --- a/src/platforms/generic_unix/main.c +++ b/src/platforms/generic_unix/main.c @@ -201,7 +201,7 @@ int main(int argc, char **argv) return EXIT_FAILURE; } - avmpack_data_init(avmpack_data, &embedded_avm_pack_info); + avmpack_data_init(avmpack_data, &embedded_avm_pack_info, (uint32_t) embedded_size); avmpack_data->data = embedded_data; // Set the name for the embedded AVM pack so it can be found by atomvm:get_start_beam/1 term escript_atom = globalcontext_make_atom(glb, ATOM_STR("\x7", "escript")); @@ -227,7 +227,9 @@ int main(int argc, char **argv) const void *startup_beam = NULL; const char *startup_module_name; uint32_t startup_beam_size; - avmpack_find_section_by_flag(avmpack_data->data, BEAM_START_FLAG, BEAM_START_FLAG, &startup_beam, &startup_beam_size, &startup_module_name); + avmpack_find_section_by_flag(avmpack_data->data, avmpack_data->size, + BEAM_START_FLAG, BEAM_START_FLAG, &startup_beam, &startup_beam_size, + &startup_module_name); if (startup_beam) { avmpack_data->in_use = true; diff --git a/src/platforms/rp2/src/main.c b/src/platforms/rp2/src/main.c index 5954aac5b3..8f146775a4 100644 --- a/src/platforms/rp2/src/main.c +++ b/src/platforms/rp2/src/main.c @@ -92,7 +92,9 @@ static int app_main() } AVM_ABORT(); } - if (!avmpack_find_section_by_flag(MAIN_AVM, BEAM_START_FLAG, BEAM_START_FLAG, &startup_beam, &startup_beam_size, &startup_module_name)) { + if (!avmpack_find_section_by_flag(MAIN_AVM, (uint32_t) (XIP_SRAM_BASE - (uintptr_t) MAIN_AVM), + BEAM_START_FLAG, BEAM_START_FLAG, &startup_beam, &startup_beam_size, + &startup_module_name)) { sleep_ms(5000); fprintf(stderr, "Fatal error: Failed to locate start module in main.avm packbeam. (Did you flash a library by mistake?)"); AVM_ABORT(); @@ -105,7 +107,8 @@ static int app_main() AVM_ABORT(); } - avmpack_data_init(&avmpack_data->base, &const_avm_pack_info); + avmpack_data_init(&avmpack_data->base, &const_avm_pack_info, + (uint32_t) (XIP_SRAM_BASE - (uintptr_t) MAIN_AVM)); avmpack_data->base.data = MAIN_AVM; avmpack_data->base.in_use = true; @@ -118,7 +121,8 @@ static int app_main() fprintf(stderr, "Memory error: Cannot allocate AVMPackData for lib.avm."); AVM_ABORT(); } - avmpack_data_init(&avmpack_data->base, &const_avm_pack_info); + avmpack_data_init(&avmpack_data->base, &const_avm_pack_info, + (uint32_t) ((uintptr_t) MAIN_AVM - (uintptr_t) LIB_AVM)); avmpack_data->base.data = LIB_AVM; avmpack_data->base.in_use = true; synclist_append(&glb->avmpack_data, &avmpack_data->base.avmpack_head); diff --git a/src/platforms/rp2/tests/test_main.c b/src/platforms/rp2/tests/test_main.c index 8aebf51d06..daaeabd113 100644 --- a/src/platforms/rp2/tests/test_main.c +++ b/src/platforms/rp2/tests/test_main.c @@ -105,7 +105,8 @@ static term avm_test_case(const char *test_module) struct ConstAVMPack *avmpack_data = malloc(sizeof(struct ConstAVMPack)); TEST_ASSERT_NOT_NULL(avmpack_data); - avmpack_data_init(&avmpack_data->base, &const_avm_pack_info); + avmpack_data_init(&avmpack_data->base, &const_avm_pack_info, + (uint32_t) (XIP_SRAM_BASE - (uintptr_t) MAIN_AVM)); avmpack_data->base.data = MAIN_AVM; avmpack_data->base.in_use = true; diff --git a/src/platforms/stm32/src/main.c b/src/platforms/stm32/src/main.c index cda86fc2e3..6decffa063 100644 --- a/src/platforms/stm32/src/main.c +++ b/src/platforms/stm32/src/main.c @@ -333,7 +333,9 @@ int main(void) port_driver_init_all(glb); nif_collection_init_all(glb); - if (!avmpack_is_valid(flashed_avm, size) || !avmpack_find_section_by_flag(flashed_avm, BEAM_START_FLAG, BEAM_START_FLAG, &startup_beam, &startup_beam_size, &startup_module_name)) { + if (!avmpack_is_valid(flashed_avm, size) + || !avmpack_find_section_by_flag(flashed_avm, size, BEAM_START_FLAG, BEAM_START_FLAG, + &startup_beam, &startup_beam_size, &startup_module_name)) { AVM_LOGE(TAG, "Invalid AVM Pack"); AVM_ABORT(); } @@ -345,7 +347,7 @@ int main(void) AVM_LOGE(TAG, "Memory error: Cannot allocate AVMPackData."); AVM_ABORT(); } - avmpack_data_init(&avmpack_data->base, &const_avm_pack_info); + avmpack_data_init(&avmpack_data->base, &const_avm_pack_info, size); avmpack_data->base.data = flashed_avm; avmpack_data->base.in_use = true; synclist_append(&glb->avmpack_data, &avmpack_data->base.avmpack_head); diff --git a/tests/test-jit_stream_flash.c b/tests/test-jit_stream_flash.c index 1dc8d3af07..7e8fbd097e 100644 --- a/tests/test-jit_stream_flash.c +++ b/tests/test-jit_stream_flash.c @@ -27,6 +27,7 @@ #include "avmpack.h" #include "context.h" +#include "defaultatoms.h" #include "globalcontext.h" #include "jit_stream_flash.h" #include "jit_stream_flash_platform.h" @@ -185,8 +186,8 @@ static uint8_t create_minimal_avmpack(void) uint32_t *sec_header = (uint32_t *) section; // Section format: size (4) + flags (4) + reserved (4) + name (null-terminated) - // Write size in big-endian (total section size including header) - uint32_t section_size = 4 + 4 + 4 + 4; // size + flags + reserved + "end\0" + // The terminator declares size 0 even though it occupies 16 bytes + uint32_t section_size = 0; sec_header[0] = __builtin_bswap32(section_size); // Write flags in big-endian @@ -202,6 +203,15 @@ static uint8_t create_minimal_avmpack(void) return 0; } +static void create_terminatorless_avmpack(void) +{ + uint8_t *pack = mock_flash + 0x100; + + const char header_str[] = "#!/usr/bin/env AtomVM\n"; + memcpy(pack, header_str, 23); + pack[23] = 0; +} + // Register AVM pack with global context static void register_test_avmpack(GlobalContext *glb) { @@ -209,7 +219,7 @@ static void register_test_avmpack(GlobalContext *glb) // Create AVMPackData struct ConstAVMPack *pack = malloc(sizeof(struct ConstAVMPack)); - avmpack_data_init(&pack->base, &const_avm_pack_info); + avmpack_data_init(&pack->base, &const_avm_pack_info, sizeof(mock_flash) - 0x100); pack->base.data = mock_flash + 0x100; pack->base.in_use = true; @@ -217,6 +227,18 @@ static void register_test_avmpack(GlobalContext *glb) synclist_append(&glb->avmpack_data, &pack->base.avmpack_head); } +static void register_terminatorless_avmpack(GlobalContext *glb) +{ + create_terminatorless_avmpack(); + + struct ConstAVMPack *pack = malloc(sizeof(struct ConstAVMPack)); + avmpack_data_init(&pack->base, &const_avm_pack_info, sizeof(mock_flash) - 0x100); + pack->base.data = mock_flash + 0x100; + pack->base.in_use = true; + + synclist_append(&glb->avmpack_data, &pack->base.avmpack_head); +} + // Test helper: create binary term with proper GC rooting static term make_binary_rooted(Context *ctx, const uint8_t *data, size_t len, term *roots, int num_roots) { @@ -836,6 +858,43 @@ static void test_stale_data_cleanup(void) fprintf(stderr, "PASS: Stale data cleanup test\n"); } +void test_no_terminator_bug(void) +{ + fprintf(stderr, "\n=== Test: Pack Without Terminator ===\n"); + + memset(mock_flash, 0x00, MOCK_FLASH_SIZE); + memset(&mock_flash[0], 0xFF, FLASH_SECTOR_SIZE); // first page with AVM + + GlobalContext *glb = globalcontext_new(); + Context *ctx = context_new(glb); + + // The only registered pack must be the broken one, otherwise a valid pack still provides an + // append position and the failure path is never reached. + register_terminatorless_avmpack(glb); + jit_stream_flash_init(glb); + + nif_function new_nif = get_nif("jit_stream_flash:new/1"); + assert(new_nif != NULL); + + term argv[1]; + argv[0] = term_from_int(10); // label count + term stream = new_nif(ctx, 1, argv); + + if (!term_is_invalid_term(stream)) { + fprintf(stderr, "FAIL: expected jit_stream_flash:new/1 to raise, got a stream\n"); + exit(1); + } + if (ctx->exception_reason != BADARG_ATOM) { + fprintf(stderr, "FAIL: expected badarg, got a different exception reason\n"); + exit(1); + } + + scheduler_terminate(ctx); + globalcontext_destroy(glb); + + fprintf(stderr, "PASS: Pack without terminator\n"); +} + int main(int argc, char **argv) { UNUSED(argc); @@ -852,6 +911,7 @@ int main(int argc, char **argv) test_esp32_crash_bug(); test_tail_corruption_bug(); test_stale_data_cleanup(); + test_no_terminator_bug(); fprintf(stderr, "\nAll tests passed!\n"); return EXIT_SUCCESS; From a0356b8de5fdab028d7776e90ac9e3bd2b63ae92 Mon Sep 17 00:00:00 2001 From: Davide Bettio Date: Mon, 22 Jun 2026 11:36:17 +0000 Subject: [PATCH 2/6] Add avmpack truncation detection at load Reject truncated or incomplete packs (e.g. an image flashed to a too-small partition) at load instead of failing late, validating by tail check or bounded walk. Signed-off-by: Davide Bettio --- .github/workflows/build-and-test.yaml | 15 ++ .github/workflows/build-linux-artifacts.yaml | 3 + CHANGELOG.md | 1 + doc/src/packbeam-format.md | 2 + src/libAtomVM/avmpack.c | 36 ++++ src/libAtomVM/avmpack.h | 31 ++- src/libAtomVM/nifs.c | 3 +- src/platforms/emscripten/src/lib/sys.c | 9 + src/platforms/esp32/components/avm_sys/sys.c | 7 +- src/platforms/esp32/main/main.c | 9 +- src/platforms/generic_unix/lib/sys.c | 2 +- src/platforms/rp2/src/main.c | 25 ++- src/platforms/stm32/src/main.c | 9 +- tests/CMakeLists.txt | 7 + tests/test-avmpack.c | 204 +++++++++++++++++++ 15 files changed, 339 insertions(+), 24 deletions(-) create mode 100644 tests/test-avmpack.c diff --git a/.github/workflows/build-and-test.yaml b/.github/workflows/build-and-test.yaml index f487ea0cf3..c92c2078ee 100644 --- a/.github/workflows/build-and-test.yaml +++ b/.github/workflows/build-and-test.yaml @@ -875,6 +875,21 @@ jobs: ulimit -c unlimited ./tests/test-structs + - name: "Test: test-avmpack with valgrind" + if: matrix.library-arch == '' + timeout-minutes: 10 + working-directory: build + run: | + ulimit -c unlimited + valgrind --error-exitcode=1 ./tests/test-avmpack + + - name: "Test: test-avmpack" + timeout-minutes: 10 + working-directory: build + run: | + ulimit -c unlimited + ./tests/test-avmpack + - name: "Test: test_etest.avm with valgrind" if: matrix.library-arch == '' timeout-minutes: 5 diff --git a/.github/workflows/build-linux-artifacts.yaml b/.github/workflows/build-linux-artifacts.yaml index ff9018cc70..7daa6e56cc 100644 --- a/.github/workflows/build-linux-artifacts.yaml +++ b/.github/workflows/build-linux-artifacts.yaml @@ -236,6 +236,7 @@ jobs: make test-heap && make test-mailbox && make test-structs && + make test-avmpack && file ./tests/test-erlang && ./tests/test-erlang -s prime_smp && file ./tests/test-enif && @@ -246,6 +247,8 @@ jobs: ./tests/test-mailbox && file ./tests/test-structs && ./tests/test-structs && + file ./tests/test-avmpack && + ./tests/test-avmpack && file ./src/AtomVM && ./src/AtomVM tests/libs/etest/test_etest.avm && ./src/AtomVM tests/libs/estdlib/test_estdlib.avm && diff --git a/CHANGELOG.md b/CHANGELOG.md index f7938d8557..8affe5286d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -88,6 +88,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 readied by a driver (e.g. an active-mode socket message) until the next event or timer tick - Fixed AVM pack lookups not finding sections placed after a data file - Fixed an out-of-bounds read crashing the VM on an AVM pack section lookup miss (e.g. `atomvm:read_priv/2` for an unpacked file) +- Fixed truncated or oversized AVM packs being accepted; they are now rejected at load instead of failing late ## [0.7.0-alpha.1] - 2026-04-06 diff --git a/doc/src/packbeam-format.md b/doc/src/packbeam-format.md index 532def5c46..b7b5033c80 100644 --- a/doc/src/packbeam-format.md +++ b/doc/src/packbeam-format.md @@ -120,6 +120,8 @@ requirement that the `Path` component of a normal file be a simple file name. Packbeam files end with a special `end` header. The `size` field of the `end` header is 0 bytes. +The `end` header is mandatory: the runtime uses it to confirm a pack is complete, and rejects a pack whose terminator is missing (e.g. a truncated flash image) instead of scanning past its data. + #### Example `end` header The following sequence of bytes encodes the `end` header: diff --git a/src/libAtomVM/avmpack.c b/src/libAtomVM/avmpack.c index 19cdf6a121..3f06521aa4 100644 --- a/src/libAtomVM/avmpack.c +++ b/src/libAtomVM/avmpack.c @@ -179,6 +179,42 @@ void *avmpack_fold( return accum; } +bool avmpack_is_complete(const void *avmpack_binary, uint32_t exact_size) +{ + // The header and every section are multiples of 4, so a complete pack's size must be too. + if (!avmpack_is_valid(avmpack_binary, exact_size) + || exact_size < AVMPACK_SIZE + AVMPACK_END_MARKER_SIZE || (exact_size & 3) != 0) { + return false; + } + + struct AVMPackSection section; + + return read_section(avmpack_binary, exact_size, exact_size - AVMPACK_END_MARKER_SIZE, §ion) + == AVMPackSectionEnd; +} + +bool avmpack_compute_size(const void *avmpack_binary, uint32_t max_size, uint32_t *real_size) +{ + if (!avmpack_is_valid(avmpack_binary, max_size)) { + return false; + } + + uint32_t offset = AVMPACK_SIZE; + struct AVMPackSection section; + enum AVMPackSectionKind kind; + while ((kind = read_section(avmpack_binary, max_size, offset, §ion)) + == AVMPackSectionRegular) { + offset += section.size; + } + + if (kind == AVMPackSectionEnd) { + *real_size = offset + AVMPACK_END_MARKER_SIZE; + return true; + } + + return false; +} + static void in_memory_avm_pack_destructor(struct AVMPackData *obj, GlobalContext *global); const struct AVMPackInfo in_memory_avm_pack_info = { diff --git a/src/libAtomVM/avmpack.h b/src/libAtomVM/avmpack.h index 26481ef930..ce88950e89 100644 --- a/src/libAtomVM/avmpack.h +++ b/src/libAtomVM/avmpack.h @@ -146,13 +146,42 @@ int avmpack_find_section_by_name(const void *avmpack_binary, uint32_t avmpack_si * @brief Returns \c true if the pointed binary starts with a valid AVM Pack header. * * @details Performs a cheap check of the 24-byte magic header only. It does not validate the - * section chain or detect truncation. + * section chain or detect truncation; use \c avmpack_is_complete or \c avmpack_compute_size + * for that. * @param avmpack_binary a pointer to an AVM Pack binary. * @param size the size of AVM Pack binary. * @returns \c true if it is a valid AVM Pack binary, \c false otherwise. */ bool avmpack_is_valid(const void *avmpack_binary, uint32_t size); +/** + * @brief Checks that an AVM Pack of an exactly known size is complete (not truncated). + * + * @details Verifies the magic header and that the pack ends exactly with the canonical \c end + * terminator section at exact_size - 16. Does not traverse the section chain, so it + * preserves lazy loading for memory-mapped files. Use this whenever the exact size of the pack is + * known (a file size, an in-memory binary). Trailing bytes after the terminator are rejected. + * @param avmpack_binary a pointer to an AVM Pack binary. + * @param exact_size the exact size in bytes of the AVM Pack. + * @returns \c true if the pack is a complete AVM Pack, \c false otherwise. + */ +bool avmpack_is_complete(const void *avmpack_binary, uint32_t exact_size); + +/** + * @brief Validates an AVM Pack within an upper-bounded region and computes its real size. + * + * @details Verifies the magic header and walks the section chain (bounded by \p max_size, + * validating every section header) until the canonical \c end terminator is reached. Use this when + * only an upper bound is known (e.g. a flash partition larger than the image); it both validates + * the pack and recovers its real size. If the terminator is not reached within \p max_size (or a + * section header is invalid), the pack is truncated or corrupt. + * @param avmpack_binary a pointer to an AVM Pack binary. + * @param max_size the size in bytes of the region containing the AVM Pack (upper bound). + * @param real_size on success, set to the real size in bytes of the AVM Pack. + * @returns \c true if a complete AVM Pack was found within \p max_size, \c false otherwise. + */ +bool avmpack_compute_size(const void *avmpack_binary, uint32_t max_size, uint32_t *real_size); + /** * @brief Fold over all the sections in an AVM Pack. * diff --git a/src/libAtomVM/nifs.c b/src/libAtomVM/nifs.c index 819eb339bf..3a70b5e537 100644 --- a/src/libAtomVM/nifs.c +++ b/src/libAtomVM/nifs.c @@ -5573,7 +5573,8 @@ static term nif_atomvm_add_avm_pack_binary(Context *ctx, int argc, term argv[]) size_t bin_size = term_binary_size(binary); - if (UNLIKELY(bin_size > UINT32_MAX || !avmpack_is_valid(term_binary_data(binary), bin_size))) { + if (UNLIKELY(bin_size > UINT32_MAX + || !avmpack_is_complete(term_binary_data(binary), (uint32_t) bin_size))) { RAISE_ERROR(BADARG_ATOM); } diff --git a/src/platforms/emscripten/src/lib/sys.c b/src/platforms/emscripten/src/lib/sys.c index 8615e0403c..60cc850511 100644 --- a/src/platforms/emscripten/src/lib/sys.c +++ b/src/platforms/emscripten/src/lib/sys.c @@ -696,6 +696,15 @@ enum OpenAVMResult sys_open_avm_from_file( return AVM_OPEN_CANNOT_OPEN; } + if (UNLIKELY(!avmpack_is_complete(data, (uint32_t) data_size))) { + if (fetch) { + emscripten_fetch_close(fetch); + } else { + free(data); + } + return AVM_OPEN_INVALID; + } + struct ConstAVMPack *const_avm = malloc(sizeof(struct ConstAVMPack)); if (IS_NULL_PTR(const_avm)) { if (fetch) { diff --git a/src/platforms/esp32/components/avm_sys/sys.c b/src/platforms/esp32/components/avm_sys/sys.c index 9ece8a26fd..15e3ac5019 100644 --- a/src/platforms/esp32/components/avm_sys/sys.c +++ b/src/platforms/esp32/components/avm_sys/sys.c @@ -450,7 +450,8 @@ enum OpenAVMResult sys_open_avm_from_file(GlobalContext *global, const char *pat if (IS_NULL_PTR(part_data)) { return AVM_OPEN_CANNOT_OPEN; } - if (UNLIKELY(!avmpack_is_valid(part_data, size))) { + uint32_t avm_size; + if (UNLIKELY(!avmpack_compute_size(part_data, size, &avm_size))) { return AVM_OPEN_INVALID; } @@ -458,7 +459,7 @@ enum OpenAVMResult sys_open_avm_from_file(GlobalContext *global, const char *pat if (IS_NULL_PTR(part_avm)) { return AVM_OPEN_FAILED_ALLOC; } - avmpack_data_init(&part_avm->base, &esp32_part_avm_pack_info, size); + avmpack_data_init(&part_avm->base, &esp32_part_avm_pack_info, avm_size); part_avm->base.data = part_data; part_avm->part_handle = part_handle; avmpack_data = &part_avm->base; @@ -490,7 +491,7 @@ enum OpenAVMResult sys_open_avm_from_file(GlobalContext *global, const char *pat return AVM_OPEN_CANNOT_READ; } - if (UNLIKELY(!avmpack_is_valid(file_data, size))) { + if (UNLIKELY(!avmpack_is_complete(file_data, size))) { free(file_data); return AVM_OPEN_INVALID; } diff --git a/src/platforms/esp32/main/main.c b/src/platforms/esp32/main/main.c index 8ee2e3c199..5f6174c1db 100644 --- a/src/platforms/esp32/main/main.c +++ b/src/platforms/esp32/main/main.c @@ -95,11 +95,12 @@ void app_main() port_driver_init_all(glb); nif_collection_init_all(glb); - if (!avmpack_is_valid(startup_avm, size)) { - ESP_LOGE(TAG, "Invalid startup avmpack. size=%u", size); + uint32_t avm_size; + if (!avmpack_compute_size(startup_avm, size, &avm_size)) { + ESP_LOGE(TAG, "Invalid or truncated startup avmpack. partition size=%u", size); AVM_ABORT(); } - if (!avmpack_find_section_by_flag(startup_avm, size, BEAM_START_FLAG, BEAM_START_FLAG, + if (!avmpack_find_section_by_flag(startup_avm, avm_size, BEAM_START_FLAG, BEAM_START_FLAG, &startup_beam, &startup_beam_size, &startup_module_name)) { ESP_LOGE(TAG, "Error: Failed to locate start module in startup partition. (Did you flash a library by mistake?)"); AVM_ABORT(); @@ -110,7 +111,7 @@ void app_main() ESP_LOGE(TAG, "Memory error: Cannot allocate AVMPackData for main.avm."); AVM_ABORT(); } - avmpack_data_init(&avmpack_data->base, &const_avm_pack_info, size); + avmpack_data_init(&avmpack_data->base, &const_avm_pack_info, avm_size); avmpack_data->base.in_use = true; avmpack_data->base.data = startup_avm; synclist_append(&glb->avmpack_data, &avmpack_data->base.avmpack_head); diff --git a/src/platforms/generic_unix/lib/sys.c b/src/platforms/generic_unix/lib/sys.c index 5145b756dd..158f1b07c5 100644 --- a/src/platforms/generic_unix/lib/sys.c +++ b/src/platforms/generic_unix/lib/sys.c @@ -435,7 +435,7 @@ enum OpenAVMResult sys_open_avm_from_file( if (IS_NULL_PTR(mapped)) { return AVM_OPEN_CANNOT_OPEN; } - if (UNLIKELY(!avmpack_is_valid(mapped->mapped, mapped->size))) { + if (UNLIKELY(!avmpack_is_complete(mapped->mapped, (uint32_t) mapped->size))) { return AVM_OPEN_INVALID; } diff --git a/src/platforms/rp2/src/main.c b/src/platforms/rp2/src/main.c index 8f146775a4..4847be7449 100644 --- a/src/platforms/rp2/src/main.c +++ b/src/platforms/rp2/src/main.c @@ -84,17 +84,18 @@ static int app_main() const void *startup_beam; const char *startup_module_name; - if (!avmpack_is_valid(MAIN_AVM, XIP_SRAM_BASE - (uintptr_t) MAIN_AVM)) { + uint32_t main_avm_size = 0; + if (!avmpack_compute_size( + MAIN_AVM, (uint32_t) (XIP_SRAM_BASE - (uintptr_t) MAIN_AVM), &main_avm_size)) { sleep_ms(5000); - fprintf(stderr, "No application loaded. Please flash your application to get started.\n"); + fprintf(stderr, "main.avm is missing or truncated. Please flash your application.\n"); if (avmpack_is_valid(LIB_AVM, (uintptr_t) MAIN_AVM - (uintptr_t) LIB_AVM)) { fprintf(stderr, "Core libraries are loaded and ready.\n"); } AVM_ABORT(); } - if (!avmpack_find_section_by_flag(MAIN_AVM, (uint32_t) (XIP_SRAM_BASE - (uintptr_t) MAIN_AVM), - BEAM_START_FLAG, BEAM_START_FLAG, &startup_beam, &startup_beam_size, - &startup_module_name)) { + if (!avmpack_find_section_by_flag(MAIN_AVM, main_avm_size, BEAM_START_FLAG, BEAM_START_FLAG, + &startup_beam, &startup_beam_size, &startup_module_name)) { sleep_ms(5000); fprintf(stderr, "Fatal error: Failed to locate start module in main.avm packbeam. (Did you flash a library by mistake?)"); AVM_ABORT(); @@ -107,25 +108,29 @@ static int app_main() AVM_ABORT(); } - avmpack_data_init(&avmpack_data->base, &const_avm_pack_info, - (uint32_t) (XIP_SRAM_BASE - (uintptr_t) MAIN_AVM)); + avmpack_data_init(&avmpack_data->base, &const_avm_pack_info, main_avm_size); avmpack_data->base.data = MAIN_AVM; avmpack_data->base.in_use = true; synclist_append(&glb->avmpack_data, &avmpack_data->base.avmpack_head); - if (avmpack_is_valid(LIB_AVM, (uintptr_t) MAIN_AVM - (uintptr_t) LIB_AVM)) { + uint32_t lib_avm_size = 0; + if (avmpack_compute_size(LIB_AVM, (uint32_t) ((uintptr_t) MAIN_AVM - (uintptr_t) LIB_AVM), + &lib_avm_size)) { avmpack_data = malloc(sizeof(struct ConstAVMPack)); if (IS_NULL_PTR(avmpack_data)) { sleep_ms(5000); fprintf(stderr, "Memory error: Cannot allocate AVMPackData for lib.avm."); AVM_ABORT(); } - avmpack_data_init(&avmpack_data->base, &const_avm_pack_info, - (uint32_t) ((uintptr_t) MAIN_AVM - (uintptr_t) LIB_AVM)); + avmpack_data_init(&avmpack_data->base, &const_avm_pack_info, lib_avm_size); avmpack_data->base.data = LIB_AVM; avmpack_data->base.in_use = true; synclist_append(&glb->avmpack_data, &avmpack_data->base.avmpack_head); + } else if (avmpack_is_valid(LIB_AVM, (uintptr_t) MAIN_AVM - (uintptr_t) LIB_AVM)) { + sleep_ms(5000); + fprintf(stderr, "lib.avm is truncated or corrupt. Please flash the core libraries.\n"); + AVM_ABORT(); } else { fprintf(stderr, "Warning: invalid lib.avm packbeam\n"); } diff --git a/src/platforms/stm32/src/main.c b/src/platforms/stm32/src/main.c index 6decffa063..b02a794dae 100644 --- a/src/platforms/stm32/src/main.c +++ b/src/platforms/stm32/src/main.c @@ -333,10 +333,11 @@ int main(void) port_driver_init_all(glb); nif_collection_init_all(glb); - if (!avmpack_is_valid(flashed_avm, size) - || !avmpack_find_section_by_flag(flashed_avm, size, BEAM_START_FLAG, BEAM_START_FLAG, + uint32_t avm_size = 0; + if (!avmpack_compute_size(flashed_avm, size, &avm_size) + || !avmpack_find_section_by_flag(flashed_avm, avm_size, BEAM_START_FLAG, BEAM_START_FLAG, &startup_beam, &startup_beam_size, &startup_module_name)) { - AVM_LOGE(TAG, "Invalid AVM Pack"); + AVM_LOGE(TAG, "Invalid or truncated AVM Pack"); AVM_ABORT(); } AVM_LOGI(TAG, "Booting file mapped at: %p, size: %lu", flashed_avm, startup_beam_size); @@ -347,7 +348,7 @@ int main(void) AVM_LOGE(TAG, "Memory error: Cannot allocate AVMPackData."); AVM_ABORT(); } - avmpack_data_init(&avmpack_data->base, &const_avm_pack_info, size); + avmpack_data_init(&avmpack_data->base, &const_avm_pack_info, avm_size); avmpack_data->base.data = flashed_avm; avmpack_data->base.in_use = true; synclist_append(&glb->avmpack_data, &avmpack_data->base.avmpack_head); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 3c7f880c49..58784127ca 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -27,6 +27,7 @@ add_executable(test-heap test-heap.c) add_executable(test-jit_stream_flash test-jit_stream_flash.c ../src/libAtomVM/jit_stream_flash.c) add_executable(test-mailbox test-mailbox.c) add_executable(test-structs test-structs.c) +add_executable(test-avmpack test-avmpack.c) target_compile_features(test-erlang PUBLIC c_std_11) target_compile_features(test-enif PUBLIC c_std_11) @@ -34,6 +35,7 @@ target_compile_features(test-heap PUBLIC c_std_11) target_compile_features(test-jit_stream_flash PUBLIC c_std_11) target_compile_features(test-mailbox PUBLIC c_std_11) target_compile_features(test-structs PUBLIC c_std_11) +target_compile_features(test-avmpack PUBLIC c_std_11) if(CMAKE_COMPILER_IS_GNUCC) target_compile_options(test-erlang PUBLIC -Wall -pedantic -Wextra -ggdb) @@ -42,6 +44,7 @@ if(CMAKE_COMPILER_IS_GNUCC) target_compile_options(test-jit_stream_flash PUBLIC -Wall -pedantic -Wextra -ggdb) target_compile_options(test-mailbox PUBLIC -Wall -pedantic -Wextra -ggdb) target_compile_options(test-structs PUBLIC -Wall -pedantic -Wextra -ggdb) + target_compile_options(test-avmpack PUBLIC -Wall -pedantic -Wextra -ggdb) endif() if(${CMAKE_SYSTEM_NAME} STREQUAL "Linux") @@ -56,6 +59,7 @@ if(${CMAKE_SYSTEM_NAME} STREQUAL "Linux") target_link_libraries(test-jit_stream_flash PRIVATE ${LIBRT}) target_link_libraries(test-mailbox PRIVATE ${LIBRT}) target_link_libraries(test-structs PRIVATE ${LIBRT}) + target_link_libraries(test-avmpack PRIVATE ${LIBRT}) else() # might also be in libc check_library_exists(c clock_gettime "" HAVE_CLOCK_GETTIME) @@ -70,6 +74,7 @@ if (MbedTLS_FOUND) target_link_libraries(test-jit_stream_flash PRIVATE MbedTLS::mbedtls) target_link_libraries(test-mailbox PRIVATE MbedTLS::mbedtls) target_link_libraries(test-structs PRIVATE MbedTLS::mbedtls) + target_link_libraries(test-avmpack PRIVATE MbedTLS::mbedtls) endif() set( @@ -97,6 +102,7 @@ target_include_directories(test-heap PRIVATE ../src/libAtomVM) target_include_directories(test-jit_stream_flash PRIVATE ../src/libAtomVM ${CMAKE_CURRENT_SOURCE_DIR}) target_include_directories(test-mailbox PRIVATE ../src/libAtomVM) target_include_directories(test-structs PRIVATE ../src/libAtomVM) +target_include_directories(test-avmpack PRIVATE ../src/libAtomVM) target_link_libraries(test-erlang PRIVATE libAtomVM libAtomVM${PLATFORM_LIB_SUFFIX}) target_link_libraries(test-enif PRIVATE libAtomVM libAtomVM${PLATFORM_LIB_SUFFIX}) target_link_libraries(test-heap PRIVATE libAtomVM libAtomVM${PLATFORM_LIB_SUFFIX}) @@ -108,6 +114,7 @@ endif() target_link_libraries(test-jit_stream_flash PRIVATE libAtomVM libAtomVM${PLATFORM_LIB_SUFFIX}) target_link_libraries(test-mailbox PRIVATE libAtomVM libAtomVM${PLATFORM_LIB_SUFFIX}) target_link_libraries(test-structs PRIVATE libAtomVM libAtomVM${PLATFORM_LIB_SUFFIX}) +target_link_libraries(test-avmpack PRIVATE libAtomVM libAtomVM${PLATFORM_LIB_SUFFIX}) # Except for XCode, also compile beams if (NOT "${CMAKE_GENERATOR}" MATCHES "Xcode") diff --git a/tests/test-avmpack.c b/tests/test-avmpack.c new file mode 100644 index 0000000000..fa1a9b90e1 --- /dev/null +++ b/tests/test-avmpack.c @@ -0,0 +1,204 @@ +/* + * This file is part of AtomVM. + * + * Copyright 2026 Davide Bettio + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 OR LGPL-2.1-or-later + */ + +#include +#include +#include +#include + +#include "avmpack.h" +#include "utils.h" + +static int failures = 0; + +#define CHECK(cond) \ + do { \ + if (!(cond)) { \ + fprintf(stderr, "FAIL line %d: %s\n", __LINE__, #cond); \ + failures++; \ + } \ + } while (0) + +static const uint8_t avmpack_header[24] = { 0x23, 0x21, 0x2f, 0x75, 0x73, 0x72, 0x2f, 0x62, 0x69, + 0x6e, 0x2f, 0x65, 0x6e, 0x76, 0x20, 0x41, 0x74, 0x6f, 0x6d, 0x56, 0x4d, 0x0a, 0x00, 0x00 }; + +static void put_u32_be(uint8_t *p, uint32_t v) +{ + p[0] = (uint8_t) (v >> 24); + p[1] = (uint8_t) (v >> 16); + p[2] = (uint8_t) (v >> 8); + p[3] = (uint8_t) v; +} + +static size_t pad4(size_t n) +{ + return (n + 3) & ~(size_t) 3; +} + +static size_t put_section( + uint8_t *buf, size_t off, const char *name, uint32_t flags, size_t content) +{ + size_t name_len = strlen(name) + 1; + size_t name_pad = pad4(name_len); + size_t content_pad = pad4(content); + put_u32_be(buf + off, (uint32_t) (12 + name_pad + content_pad)); + put_u32_be(buf + off + 4, flags); + put_u32_be(buf + off + 8, 0); + memcpy(buf + off + 12, name, name_len); + memset(buf + off + 12 + name_len, 0, name_pad - name_len); + memset(buf + off + 12 + name_pad, 0, content_pad); + + return off + 12 + name_pad + content_pad; +} + +static void *count_sections(void *accum, const void *section_ptr, uint32_t section_size, + const void *beam_ptr, uint32_t flags, const char *section_name) +{ + UNUSED(section_ptr); + UNUSED(section_size); + UNUSED(beam_ptr); + UNUSED(flags); + UNUSED(section_name); + + return (void *) ((uintptr_t) accum + 1); +} + +static size_t put_end(uint8_t *buf, size_t off) +{ + put_u32_be(buf + off, 0); + put_u32_be(buf + off + 4, 0); + put_u32_be(buf + off + 8, 0); + memcpy(buf + off + 12, "end", 4); + + return off + 16; +} + +int main(void) +{ + uint8_t buf[4096]; + const void *ptr = NULL; + uint32_t sz = 0; + const char *name = NULL; + uint32_t real_size = 0; + + // --- A valid pack: header + one BEAM section + terminator --- + memset(buf, 0, sizeof(buf)); + memcpy(buf, avmpack_header, 24); + size_t off = put_section(buf, 24, "mymod.beam", BEAM_START_FLAG, 16); + uint32_t pack_size = (uint32_t) put_end(buf, off); + + CHECK(avmpack_is_valid(buf, pack_size)); + CHECK(avmpack_is_complete(buf, pack_size)); + CHECK(avmpack_compute_size(buf, sizeof(buf), &real_size) && real_size == pack_size); + CHECK(avmpack_find_section_by_name(buf, pack_size, "mymod.beam", &ptr, &sz) == 1); + CHECK(ptr == buf + 24 + 12 + 12); // header + section header + padded "mymod.beam\0" + CHECK(sz == 16); // the payload size, not the whole section size + CHECK(avmpack_find_section_by_name(buf, pack_size, "missing", &ptr, &sz) == 0); + CHECK(avmpack_find_section_by_flag( + buf, pack_size, BEAM_START_FLAG, BEAM_START_FLAG, &ptr, &sz, &name) + == 1); + CHECK(name != NULL && strcmp(name, "mymod.beam") == 0); + CHECK(sz == 16); + // find_by_flag for the terminator still returns it (the JIT cache relies on this) + CHECK(avmpack_find_section_by_flag( + buf, pack_size, END_OF_FILE_MASK, END_OF_FILE, &ptr, &sz, &name) + == 1); + CHECK(name != NULL && strcmp(name, "end") == 0); + + // --- Sections after a data file (flags 0) stay reachable --- + memset(buf, 0, sizeof(buf)); + memcpy(buf, avmpack_header, 24); + off = put_section(buf, 24, "app.beam", BEAM_START_FLAG, 16); + off = put_section(buf, off, "app/priv/a.txt", 0, 8); + off = put_section(buf, off, "app/priv/b.txt", 0, 8); + pack_size = (uint32_t) put_end(buf, off); + CHECK(avmpack_find_section_by_name(buf, pack_size, "app/priv/b.txt", &ptr, &sz) == 1); + CHECK(sz == 8); + CHECK(avmpack_fold(NULL, buf, pack_size, count_sections) == (void *) (uintptr_t) 3); + + // --- Truncated pack: section present, terminator chopped off, 0xFF tail --- + memset(buf, 0xFF, sizeof(buf)); + memcpy(buf, avmpack_header, 24); + put_section(buf, 24, "mymod.beam", BEAM_CODE_FLAG, 16); + CHECK(avmpack_compute_size(buf, sizeof(buf), &real_size) == false); + CHECK(avmpack_find_section_by_name(buf, sizeof(buf), "missing", &ptr, &sz) == 0); + // a hit before the truncation is still reachable + CHECK(avmpack_find_section_by_name(buf, sizeof(buf), "mymod.beam", &ptr, &sz) == 1); + // a file cut exactly at the section end has no tail terminator + CHECK(avmpack_is_complete(buf, 24 + 12 + 12 + 16) == false); + + // --- Oversized section size --- + memset(buf, 0xFF, sizeof(buf)); + memcpy(buf, avmpack_header, 24); + put_u32_be(buf + 24, 0x10000); // claims 64 KiB, far past the bounded view + put_u32_be(buf + 24 + 4, 0); + put_u32_be(buf + 24 + 8, 0); + memcpy(buf + 24 + 12, "x", 2); + CHECK(avmpack_find_section_by_name(buf, 64, "anything", &ptr, &sz) == 0); + CHECK(avmpack_compute_size(buf, 64, &real_size) == false); + + // --- 0x00-filled region must not be mistaken for the terminator --- + memset(buf, 0, sizeof(buf)); + memcpy(buf, avmpack_header, 24); + CHECK(avmpack_compute_size(buf, sizeof(buf), &real_size) == false); + CHECK(avmpack_find_section_by_flag( + buf, sizeof(buf), END_OF_FILE_MASK, END_OF_FILE, &ptr, &sz, &name) + == 0); + + // --- Misaligned pack base (e.g. atomvm:add_avm_pack_binary/2 with a sub-binary) --- + _Alignas(uint32_t) uint8_t storage[512 + 1]; + uint8_t *pack = storage + 1; + memset(storage, 0xFF, sizeof(storage)); + memcpy(pack, avmpack_header, 24); + off = put_section(pack, 24, "mymod.beam", BEAM_START_FLAG, 16); + pack_size = (uint32_t) put_end(pack, off); + CHECK(avmpack_is_complete(pack, pack_size)); + CHECK(avmpack_compute_size(pack, sizeof(storage) - 1, &real_size) && real_size == pack_size); + CHECK(avmpack_find_section_by_name(pack, pack_size, "mymod.beam", &ptr, &sz) == 1 && sz == 16); + CHECK(avmpack_find_section_by_flag( + pack, pack_size, END_OF_FILE_MASK, END_OF_FILE, &ptr, &sz, &name) + == 1); + CHECK(avmpack_find_section_by_name(pack, pack_size, "missing", &ptr, &sz) == 0); + + // --- Section name with no NUL terminator inside the section --- + memset(buf, 'A', sizeof(buf)); + memcpy(buf, avmpack_header, 24); + put_u32_be(buf + 24, 64); // in-range size, but the name region is all 'A' + put_u32_be(buf + 24 + 4, 0); + put_u32_be(buf + 24 + 8, 0); + CHECK(avmpack_find_section_by_name(buf, 24 + 64 + 16, "x", &ptr, &sz) == 0); + + // --- Misaligned section size --- + memset(buf, 0xFF, sizeof(buf)); + memcpy(buf, avmpack_header, 24); + put_u32_be(buf + 24, 17); + put_u32_be(buf + 24 + 4, 0); + put_u32_be(buf + 24 + 8, 0); + memcpy(buf + 24 + 12, "x", 2); + CHECK(avmpack_compute_size(buf, sizeof(buf), &real_size) == false); + + if (failures == 0) { + fprintf(stderr, "All avmpack tests passed!\n"); + return 0; + } + fprintf(stderr, "%d avmpack test(s) failed.\n", failures); + + return 1; +} From 3ae2a3482bc375a558c639e7d34309557300f228 Mon Sep 17 00:00:00 2001 From: Davide Bettio Date: Mon, 22 Jun 2026 11:53:56 +0000 Subject: [PATCH 3/6] Make ESP32 build reject an oversized boot.avm Fail at configure time when boot.avm does not fit its partition. Signed-off-by: Davide Bettio --- CHANGELOG.md | 2 ++ src/platforms/esp32/CMakeLists.txt | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8affe5286d..6871540a97 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 from ref ticks, `TERM_BOXED_REFERENCE_PROCESS_SIZE` for process references (aliases), or `TERM_BOXED_REFERENCE_MAX_SIZE` to fit any reference. `REF_SIZE` still expands to the short reference size, but now emits a compiler warning +- ESP32 builds now fail at configure time when `boot.avm` does not fit its partition, instead of + producing an image that is silently truncated at flash time ### Removed - Removed `ahttp_client` support for obsolete line folding (RFC 9112 §5.2); folded header and diff --git a/src/platforms/esp32/CMakeLists.txt b/src/platforms/esp32/CMakeLists.txt index 2e2a86b2e6..433f415eb9 100644 --- a/src/platforms/esp32/CMakeLists.txt +++ b/src/platforms/esp32/CMakeLists.txt @@ -139,6 +139,15 @@ if (NOT ("${BOOT_LIBS}" STREQUAL "NONE")) Consult https://doc.atomvm.org/main/build-instructions.html for build instructions.") else() partition_table_get_partition_info(lib_offset "--partition-name boot.avm" "offset") + partition_table_get_partition_info(lib_size "--partition-name boot.avm" "size") + # Re-run the size check when boot.avm changes without a manual reconfigure + set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS "${BOOT_LIB_PATH}") + file(SIZE "${BOOT_LIB_PATH}" boot_avm_size) + math(EXPR boot_part_size "${lib_size}") + if (boot_avm_size GREATER boot_part_size) + message(FATAL_ERROR "${BOOT_LIBS} (${boot_avm_size} bytes) does not fit in the boot.avm \ + partition (${boot_part_size} bytes). Enlarge the boot.avm partition or reduce the boot libraries.") + endif() esptool_py_flash_target_image(flash boot.avm "${lib_offset}" "${BOOT_LIB_PATH}") endif() endif() From 8b21165d5abd11ae750f2d60789801a2c24f80af Mon Sep 17 00:00:00 2001 From: Davide Bettio Date: Tue, 21 Jul 2026 18:03:57 +0000 Subject: [PATCH 4/6] Change read_priv error on a corrupt avmpack entry A priv entry whose length prefix does not fit its own section was skipped, so a corrupt pack was indistinguishable from a missing file. Raise invalid_avm instead. A genuine miss still returns undefined. Read the length prefix unaligned: an in-place pack may sit at any byte offset and the direct load faults Xtensa/Cortex-M0+. Signed-off-by: Davide Bettio --- CHANGELOG.md | 3 +++ libs/eavmlib/src/atomvm.erl | 7 +++-- src/libAtomVM/defaultatoms.def | 2 ++ src/libAtomVM/nifs.c | 48 +++++++++++++++++++-------------- tests/test-avmpack.c | 49 ++++++++++++++++++++++++++++++++++ 5 files changed, 87 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6871540a97..1ce2d0477e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,6 +61,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 reference size, but now emits a compiler warning - ESP32 builds now fail at configure time when `boot.avm` does not fit its partition, instead of producing an image that is silently truncated at flash time +- `atomvm:read_priv/2` now raises `invalid_avm` when the requested resource is present but its + recorded size does not fit the AVM pack section holding it, instead of reporting the resource as + missing. A resource that is genuinely absent still returns `undefined` ### Removed - Removed `ahttp_client` support for obsolete line folding (RFC 9112 §5.2); folded header and diff --git a/libs/eavmlib/src/atomvm.erl b/libs/eavmlib/src/atomvm.erl index d5acf31224..872fe93372 100644 --- a/libs/eavmlib/src/atomvm.erl +++ b/libs/eavmlib/src/atomvm.erl @@ -161,11 +161,14 @@ rand_bytes(_Len) -> %%----------------------------------------------------------------------------- %% @param App application name. %% @param Path path to the resource. -%% @returns Binary containing the resource content. +%% @returns Binary containing the resource content, or `undefined' if it is not in any loaded +%% AVM pack. %% @doc This function allows to fetch priv/ resources content. +%% Raises `invalid_avm' if the resource is present but its recorded size does not fit +%% the AVM pack section holding it. %% @end %%----------------------------------------------------------------------------- --spec read_priv(App :: atom(), Path :: list()) -> binary(). +-spec read_priv(App :: atom(), Path :: list()) -> binary() | undefined. read_priv(_App, _Path) -> erlang:nif_error(undefined). diff --git a/src/libAtomVM/defaultatoms.def b/src/libAtomVM/defaultatoms.def index 8911bc90a0..bfa4b968a4 100644 --- a/src/libAtomVM/defaultatoms.def +++ b/src/libAtomVM/defaultatoms.def @@ -230,3 +230,5 @@ X(REPLY_DEMONITOR_ATOM, "\xF", "reply_demonitor") X(REPLY_ATOM, "\x5", "reply") X(TAG_ATOM, "\x3", "tag") X(PRIORITY_ATOM, "\x8", "priority") + +X(INVALID_AVM_ATOM, "\xB", "invalid_avm") diff --git a/src/libAtomVM/nifs.c b/src/libAtomVM/nifs.c index 3a70b5e537..332a5a8dde 100644 --- a/src/libAtomVM/nifs.c +++ b/src/libAtomVM/nifs.c @@ -5837,6 +5837,7 @@ static term nif_atomvm_read_priv(Context *ctx, int argc, term argv[]) uint32_t size; struct ListHead *item; term result = UNDEFINED_ATOM; + bool invalid_pack = false; struct ListHead *avmpack_data = synclist_rdlock(&glb->avmpack_data); LIST_FOR_EACH (item, avmpack_data) { struct AVMPackData *avmpack_data = GET_LIST_ENTRY(item, struct AVMPackData, avmpack_head); @@ -5844,32 +5845,39 @@ static term nif_atomvm_read_priv(Context *ctx, int argc, term argv[]) avmpack_data->in_use = true; if (avmpack_find_section_by_name( avmpack_data->data, avmpack_data->size, complete_path, &bin_data, &size)) { - // Bound the length prefix to the section payload against a corrupt file size. - if (size >= sizeof(uint32_t)) { - uint32_t file_size = READ_32_ALIGNED((uint32_t *) bin_data); - if (file_size <= size - sizeof(uint32_t)) { - free(complete_path); - complete_path = NULL; - if (UNLIKELY(memory_ensure_free_opt( - ctx, TERM_BOXED_REFC_BINARY_SIZE, MEMORY_CAN_SHRINK) - != MEMORY_GC_OK)) { - avmpack_data->in_use = prev_in_use; - synclist_unlock(&glb->avmpack_data); - RAISE_ERROR(OUT_OF_MEMORY_ATOM); - } - result = term_from_const_binary(((uint8_t *) bin_data) + sizeof(uint32_t), - file_size, &ctx->heap, ctx->global); - break; - } + if (UNLIKELY(size < sizeof(uint32_t))) { + avmpack_data->in_use = prev_in_use; + invalid_pack = true; + break; } - avmpack_data->in_use = prev_in_use; - } else { - avmpack_data->in_use = prev_in_use; + uint32_t file_size = READ_32_UNALIGNED(bin_data); + if (UNLIKELY(file_size > size - sizeof(uint32_t))) { + avmpack_data->in_use = prev_in_use; + invalid_pack = true; + break; + } + free(complete_path); + complete_path = NULL; + if (UNLIKELY(memory_ensure_free_opt(ctx, TERM_BOXED_REFC_BINARY_SIZE, MEMORY_CAN_SHRINK) + != MEMORY_GC_OK)) { + avmpack_data->in_use = prev_in_use; + synclist_unlock(&glb->avmpack_data); + RAISE_ERROR(OUT_OF_MEMORY_ATOM); + } + result = term_from_const_binary( + ((uint8_t *) bin_data) + sizeof(uint32_t), file_size, &ctx->heap, ctx->global); + break; } + avmpack_data->in_use = prev_in_use; } synclist_unlock(&glb->avmpack_data); free(complete_path); + + if (UNLIKELY(invalid_pack)) { + RAISE_ERROR(INVALID_AVM_ATOM); + } + return result; } diff --git a/tests/test-avmpack.c b/tests/test-avmpack.c index fa1a9b90e1..3c979c6f97 100644 --- a/tests/test-avmpack.c +++ b/tests/test-avmpack.c @@ -21,9 +21,18 @@ #include #include #include +#include #include #include "avmpack.h" +#include "context.h" +#include "defaultatoms.h" +#include "globalcontext.h" +#include "memory.h" +#include "nifs.h" +#include "scheduler.h" +#include "synclist.h" +#include "term.h" #include "utils.h" static int failures = 0; @@ -90,6 +99,44 @@ static size_t put_end(uint8_t *buf, size_t off) return off + 16; } +static void test_read_priv_rejects_corrupt_entry(void) +{ + static uint32_t pack_words[64]; + uint8_t *pack = (uint8_t *) pack_words; + + memset(pack, 0, sizeof(pack_words)); + memcpy(pack, avmpack_header, 24); + size_t off = put_section(pack, 24, "myapp/priv/foo.txt", 4, 8); + uint32_t pack_size = (uint32_t) put_end(pack, off); + + // 8 payload bytes hold at most a 4 byte file, claim 100 + put_u32_be(pack + 24 + 12 + 20, 100); + + GlobalContext *glb = globalcontext_new(); + Context *ctx = context_new(glb); + + struct ConstAVMPack *avm = malloc(sizeof(struct ConstAVMPack)); + avmpack_data_init(&avm->base, &const_avm_pack_info, pack_size); + avm->base.data = pack; + avm->base.in_use = true; + synclist_append(&glb->avmpack_data, &avm->base.avmpack_head); + + const struct Nif *nif = nifs_get("atomvm:read_priv/2"); + CHECK(nif != NULL); + + term argv[2]; + argv[0] = globalcontext_make_atom(glb, ATOM_STR("\x5", "myapp")); + CHECK(memory_ensure_free(ctx, term_binary_heap_size(7)) == MEMORY_GC_OK); + argv[1] = term_from_literal_binary((const uint8_t *) "foo.txt", 7, &ctx->heap, glb); + + term result = nif->nif_ptr(ctx, 2, argv); + CHECK(term_is_invalid_term(result)); + CHECK(ctx->exception_reason == INVALID_AVM_ATOM); + + scheduler_terminate(ctx); + globalcontext_destroy(glb); +} + int main(void) { uint8_t buf[4096]; @@ -194,6 +241,8 @@ int main(void) memcpy(buf + 24 + 12, "x", 2); CHECK(avmpack_compute_size(buf, sizeof(buf), &real_size) == false); + test_read_priv_rejects_corrupt_entry(); + if (failures == 0) { fprintf(stderr, "All avmpack tests passed!\n"); return 0; From c4bf4411b7620807293004f4ad0c0053ce4284df Mon Sep 17 00:00:00 2001 From: Davide Bettio Date: Tue, 21 Jul 2026 22:29:13 +0000 Subject: [PATCH 5/6] Fix mapped file leak on invalid avmpack open The generic_unix invalid-pack rejection in sys_open_avm_from_file returned without releasing the mapping, leaking it and its file descriptor. Pre-existing, but truncated packs now take this path. Signed-off-by: Davide Bettio --- CHANGELOG.md | 1 + src/platforms/generic_unix/lib/sys.c | 1 + 2 files changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ce2d0477e..390f0c8c69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -94,6 +94,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed AVM pack lookups not finding sections placed after a data file - Fixed an out-of-bounds read crashing the VM on an AVM pack section lookup miss (e.g. `atomvm:read_priv/2` for an unpacked file) - Fixed truncated or oversized AVM packs being accepted; they are now rejected at load instead of failing late +- Fixed generic_unix leaking the file mapping when rejecting an invalid `.avm` file ## [0.7.0-alpha.1] - 2026-04-06 diff --git a/src/platforms/generic_unix/lib/sys.c b/src/platforms/generic_unix/lib/sys.c index 158f1b07c5..d0f044c3ed 100644 --- a/src/platforms/generic_unix/lib/sys.c +++ b/src/platforms/generic_unix/lib/sys.c @@ -436,6 +436,7 @@ enum OpenAVMResult sys_open_avm_from_file( return AVM_OPEN_CANNOT_OPEN; } if (UNLIKELY(!avmpack_is_complete(mapped->mapped, (uint32_t) mapped->size))) { + mapped_file_close(mapped); return AVM_OPEN_INVALID; } From 67020086e04928006753e209f72133e838c49ddb Mon Sep 17 00:00:00 2001 From: Davide Bettio Date: Wed, 22 Jul 2026 11:20:00 +0000 Subject: [PATCH 6/6] Fix rp2 avmpack scan bound to end of flash The XIP_SRAM_BASE bound spans past the flash, so a corrupt section size could steer bounded header reads into unmapped address space. Signed-off-by: Davide Bettio --- src/platforms/rp2/src/main.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/platforms/rp2/src/main.c b/src/platforms/rp2/src/main.c index 4847be7449..0bde2a5e47 100644 --- a/src/platforms/rp2/src/main.c +++ b/src/platforms/rp2/src/main.c @@ -85,8 +85,9 @@ static int app_main() const char *startup_module_name; uint32_t main_avm_size = 0; - if (!avmpack_compute_size( - MAIN_AVM, (uint32_t) (XIP_SRAM_BASE - (uintptr_t) MAIN_AVM), &main_avm_size)) { + if (!avmpack_compute_size(MAIN_AVM, + (uint32_t) (XIP_BASE + PICO_FLASH_SIZE_BYTES - (uintptr_t) MAIN_AVM), + &main_avm_size)) { sleep_ms(5000); fprintf(stderr, "main.avm is missing or truncated. Please flash your application.\n"); if (avmpack_is_valid(LIB_AVM, (uintptr_t) MAIN_AVM - (uintptr_t) LIB_AVM)) {