Fail read_priv 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.

Signed-off-by: Davide Bettio <davide@uninstall.it>

Author: bettio

CHANGELOG.md

@@ -59,6 +59,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

libs/eavmlib/src/atomvm.erl

@@ -161,8 +161,11 @@ 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().

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")

src/libAtomVM/nifs.c

@@ -5764,39 +5764,48 @@ 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);
         bool prev_in_use = avmpack_data->in_use;
         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;
-                }
+            // The length prefix must fit the section payload, otherwise the pack is corrupt.
+            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_ALIGNED((uint32_t *) 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;
 }
 

tests/test-avmpack.c

@@ -21,9 +21,18 @@
 #include <stdbool.h>
 #include <stdint.h>
 #include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 
 #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"
 
 static int failures = 0;
 
@@ -76,6 +85,46 @@ static size_t put_end(uint8_t *buf, size_t off)
     return off + 16;
 }
 
+// A priv entry whose length prefix does not fit its section must be reported as a corrupt pack,
+// not as a missing file.
+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];
@@ -152,6 +201,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;