Skip to content

Commit 91eba66

Browse files
committed
libretro-db/rmsgpack: fix buffer overflow in RDT_BINARY read_into path
In rmsgpack_dom_read_into's RDT_BINARY case, the caller's buffer capacity (passed in via *uint_value) was being overwritten with the source field's length before the bound-check min() was evaluated: *uint_value = value->val.binary.len; /* clobber */ min_len = (value->val.binary.len > *uint_value) ? *uint_value : value->val.binary.len; memcpy(buff_value, value->val.binary.buff, min_len); After the clobber, the comparison collapses to 'len > len' which is always false, so min_len always equals the source length and the memcpy copies the full on-disk binary blob regardless of the caller's buffer size. With an attacker-crafted libretro-db file (or a corrupted one), this is a heap/stack buffer overflow sized by untrusted input. The sibling RDT_STRING case had the correct order (compute min_len first using source_len and *uint_value, then write min_len back to *uint_value). Apply the same ordering to RDT_BINARY. Reachability: no caller of rmsgpack_dom_read_into currently requests an RDT_BINARY field - the two call sites in libretrodb.c request only RDT_UINT and RDT_STRING. So the bug is latent: fixing it now means that whenever someone legitimately adds an RDT_BINARY field to a db schema and wires up a read_into call, they won't silently inherit an overflow. Behavior change: none for callers whose buffer is at least as large as the on-disk value (min_len collapses to source length either way). Overflow prevented when the on-disk length exceeds caller's buffer; the caller's *uint_value is set to the actual number of bytes copied, mirroring the RDT_STRING contract. Thread-safety: none affected. The function operates on a stack-local va_list and a stack-local map tree; writes go through caller-provided pointers. No shared state.
1 parent 27ab9e7 commit 91eba66

1 file changed

Lines changed: 7 additions & 1 deletion

File tree

libretro-db/rmsgpack_dom.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -506,9 +506,15 @@ int rmsgpack_dom_read_into(intfstream_t *fd, ...)
506506
case RDT_BINARY:
507507
buff_value = va_arg(ap, char *);
508508
uint_value = va_arg(ap, uint64_t *);
509-
*uint_value = value->val.binary.len;
509+
/* *uint_value is the caller's buffer capacity on entry
510+
* and the number of bytes copied on exit. Compute the
511+
* clamped copy length BEFORE overwriting *uint_value,
512+
* otherwise the bound check collapses to len > len and
513+
* the memcpy can overrun the caller's buffer with
514+
* attacker-controlled length from the db file. */
510515
min_len = (value->val.binary.len > *uint_value) ?
511516
*uint_value : value->val.binary.len;
517+
*uint_value = min_len;
512518

513519
memcpy(buff_value, value->val.binary.buff, (size_t)min_len);
514520
break;

0 commit comments

Comments
 (0)