From 27f857e37bd4a2872574dc089f88ab2831d7a4a6 Mon Sep 17 00:00:00 2001 From: Acts1631 Date: Mon, 6 Jul 2026 10:41:17 -0400 Subject: [PATCH] otr: fix off-by-one in reassembly buffer growth check (heap overflow) In enqueue_otr_fragment(), once a ?OTR: reassembly is open, each continuation fragment is appended to opc->full_msg and the buffer is grown only if there isn't enough room: if (msg_len > (opc->msg_size - opc->msg_len)) { realloc(...); } memcpy(opc->full_msg + opc->msg_len, msg, msg_len); opc->msg_len += msg_len; opc->full_msg[opc->msg_len] = '\0'; The comparison uses '>' instead of '>='. When a fragment's length is exactly equal to the remaining space (opc->msg_size - opc->msg_len), the condition is false, so no realloc happens; the memcpy itself still fits, but the following NUL-terminator write at opc->full_msg[opc->msg_len] lands exactly one byte past the end of the allocation, corrupting the adjacent heap chunk. This is remotely reachable the same way as the other reassembly bugs in this file: any user who can send the victim a private message can drive the running remaining-space counter to land on an exact match (remaining space grows by a small, attacker-observable amount on every realloc, and fragment lengths are fully attacker controlled), then send one more fragment of that exact length to trigger the overflow. Fix the comparison to '>=' so the buffer is grown whenever there isn't room for both the fragment bytes and the terminator. --- src/otr/otr.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/otr/otr.c b/src/otr/otr.c index 8602f2a3..94cb65aa 100644 --- a/src/otr/otr.c +++ b/src/otr/otr.c @@ -615,7 +615,13 @@ static enum otr_msg_status enqueue_otr_fragment(const char *msg, struct otr_peer } if (opc->full_msg) { - if (msg_len > (opc->msg_size - opc->msg_len)) { + /* Grow the buffer unless it already has room for msg_len bytes + * *plus* the NUL terminator written below. Using '>' here (instead + * of '>=') is an off-by-one: when msg_len exactly equals the + * remaining space, no realloc happens, but opc->full_msg[opc->msg_len] + * is still written after copying, one byte past the end of the + * allocation. */ + if (msg_len >= (opc->msg_size - opc->msg_len)) { char *tmp_ptr; /* Realloc memory if there is not enough space. */