mirror of
https://github.com/irssi/irssi.git
synced 2026-08-08 19:30:15 +02:00
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.
This commit is contained in:
parent
0b7fae5652
commit
27f857e37b
1 changed files with 7 additions and 1 deletions
|
|
@ -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. */
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue