Low 26 Bitcoin Committed Aug 10, 2026 · 02:57 UTC

qr: split datastream data into its own allocation, reorder alloc/free calls

43640f8c View source ↗

Public commit record

What the developer wrote

Authored by Jon Griffiths

73/100 · Adequate

qr: split datastream data into its own allocation, reorder alloc/free calls

Two 8kb allocations are more likely to succeed than a single 16kb
allocation, when dram is fragmented.

Also reorder alloc/free calls into matched reverse pairs to reduce fragmentation.

✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context

The short version

What changed, and why it matters

This change rewrites how a small QR-code scanning library allocates memory. Instead of one big 16 KB block, it now uses two separate 8 KB blocks, and it frees them in the reverse order they were allocated. The stated goal is to avoid memory-allocation failures on a small embedded device when memory is fragmented. The patch itself does not fix a known exploitable bug, but it touches memory allocation and pointer handling in code that processes untrusted QR images, so it has defensive-security relevance.

Recommended action

Review whether the new `qr_data->ds->data` allocation failure path is handled safely on production builds where `JADE_ASSERT` may not abort execution. Verify that `quirc_destroy` does not dereference `qr_data->q` after the datastream is freed, and confirm the new pointer is always initialized before `quirc_decode` is called. Consider adding an explicit NULL check and graceful cleanup for the second allocation.

Security signals we found

Memory allocation pattern changed from single large allocation to two smaller allocations

Allocation/free ordering changed to LIFO-style reverse pairing

Pointer field added to struct that previously held an inline array

Manual memset updated to clear new pointer and separately allocated buffer

No explicit NULL check or fallback added for the new ds->data allocation beyond JADE_ASSERT

Technical analysis

Evidence from the diff

The commit splits the datastream struct so its 8 KB data payload buffer is allocated separately from the rest of the struct. It also reorders allocation/free calls so allocations and frees are matched in reverse order (LIFO). The decode.c change updates memset to clear the new pointer field and the separately-allocated buffer. The qrscan.c changes add a second JADE_MALLOC_PREFER_DRAM call for ds->data and move quirc_destroy after the datastream free. No bounds checks or error handling for the new allocation are added beyond the existing JADE_ASSERT.

Changed components

components/esp32-quirc/lib/decode.ccomponents/esp32-quirc/lib/quirc.hmain/qrscan.c

Inspect captured patch +24 / −15

### components/esp32-quirc/lib/decode.c
@@ -954,7 +954,10 @@ quirc_decode_error_t quirc_decode(const struct quirc_code *code,
     return err;
   }
 
-  memset(ds, 0, sizeof(*ds));
+  memset(ds->raw, 0, sizeof(ds->raw));
+  ds->data_bits = 0;
+  ds->ptr = 0;
+  memset(ds->data, 0, QUIRC_MAX_PAYLOAD * sizeof(uint8_t));
 
   read_data(code, data, ds);
   err = codestream_ecc(data, ds);

### components/esp32-quirc/lib/quirc.h
@@ -166,7 +166,7 @@ extern "C"
     int data_bits;
     int ptr;
 
-    uint8_t data[QUIRC_MAX_PAYLOAD];
+    uint8_t *data; // Must be QUIRC_MAX_PAYLOAD in bytes
   } __attribute__((aligned(8)));
 
   /* Return the number of QR-codes identified in the last processed
@@ -187,4 +187,4 @@ extern "C"
 }
 #endif
 
-#endif
\ No newline at end of file
+#endif

### main/qrscan.c
@@ -128,8 +128,6 @@ bool scan_qr(const size_t width, const size_t height, const uint8_t* data, const
     // Create the quirc structs
     qr_data->q = quirc_new();
     JADE_ASSERT(qr_data->q);
-    qr_data->ds = JADE_MALLOC_PREFER_DRAM(sizeof(struct datastream));
-    JADE_ASSERT(qr_data->ds);
 
     // Also correctly size the internal image buffer since we know the size of the camera images.
     const uint16_t scan_width = min_u16(CAMERA_IMAGE_WIDTH, CAMERA_IMAGE_HEIGHT) - SCAN_MARGIN;
@@ -140,13 +138,18 @@ bool scan_qr(const size_t width, const size_t height, const uint8_t* data, const
     JADE_LOGE("SCAN WIDTH: %u", scan_width);
     JADE_LOGE("SCAN HEIGHT: %u", scan_width);
 
+    qr_data->ds = JADE_MALLOC_PREFER_DRAM(sizeof(struct datastream));
+    qr_data->ds->data = JADE_MALLOC_PREFER_DRAM(QUIRC_MAX_PAYLOAD * sizeof(uint8_t));
+
     const bool ret = qr_recognize(width, height, data, len, qr_data);
 
     // Destroy the quirc structs created above
-    quirc_destroy(qr_data->q);
-    qr_data->q = NULL;
+    free(qr_data->ds->data);
+    qr_data->ds->data = NULL;
     free(qr_data->ds);
     qr_data->ds = NULL;
+    quirc_destroy(qr_data->q);
+    qr_data->q = NULL;
 
     // Any scanned qr code will be in the qr_data passed
     return ret && qr_data->len > 0;
@@ -171,8 +174,6 @@ bool jade_camera_scan_qr(
     JADE_ASSERT(!qr_data->q);
     qr_data->q = quirc_new();
     JADE_ASSERT(qr_data->q);
-    qr_data->ds = JADE_MALLOC_PREFER_DRAM(sizeof(struct datastream));
-    JADE_ASSERT(qr_data->ds);
 
     // Also correctly size the internal image buffer since we know the size of the camera images.
     // This image buffer is then reused for every camera image frame processed.
@@ -184,25 +185,30 @@ bool jade_camera_scan_qr(
     JADE_LOGE("SCAN WIDTH: %u", scan_width);
     JADE_LOGE("SCAN HEIGHT: %u", scan_width);
 
+    qr_data->ds = JADE_MALLOC_PREFER_DRAM(sizeof(struct datastream));
+    qr_data->ds->data = JADE_MALLOC_PREFER_DRAM(QUIRC_MAX_PAYLOAD * sizeof(uint8_t));
+
     // Run the camera task trying to interpet frames as qr-codes
     const bool show_camera_ui = true;
     const bool show_click_button = false;
     gui_activity_t* camera_act = NULL;
     jade_camera_process_images(qr_recognize, qr_data, show_camera_ui, text_label, show_click_button, qr_guide_type,
         help_url, qr_data->progress_bar, &camera_act);
 
-    // Destroy the quirc structs created above
-    quirc_destroy(qr_data->q);
-    qr_data->q = NULL;
-    free(qr_data->ds);
-    qr_data->ds = NULL;
-
     // Destroy the camera activity that was created by the camera task
     // and restore the previous activity.
     if (camera_act) {
         gui_destroy_current_activity(camera_act, prev_act);
     }
 
+    // Destroy the quirc structs created above
+    free(qr_data->ds->data);
+    qr_data->ds->data = NULL;
+    free(qr_data->ds);
+    qr_data->ds = NULL;
+    quirc_destroy(qr_data->q);
+    qr_data->q = NULL;
+
     // Any scanned qr code will be in the qr_data passed
     return qr_data->len > 0;
 #else // CONFIG_HAS_CAMERA

Risk score

Why this scored 26/100

Our methodology →

Potential impact 5/30

Exploitability 3/25

Stealth signal 4/15

Affected reach 5/15

Confidence 6/10

Evidence quality 3/5

Human-validated context

Community notes

Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.

No validated notes yet.

The AI analysis stands alone for now. Submit a note if you can add evidence or important context.

What the developer wrote

What changed, and why it matters

Security signals we found

Evidence from the diff

Changed components

Why this scored 26/100

Community notes

Related watches

fmt: fix and prevent future use of unsupported format spec "zu"

otp: release md context on failure and clean temporary buffers

identity: clean temporary hash buffer