Skip to content

Commit

Permalink
Fix/improve buffer overflow in MSG_ReadBits/MSG_WriteBits (#937)
Browse files Browse the repository at this point in the history 
Prevent reading past end of message in MSG_ReadBits. If read past
end of msg->data buffer (16348 bytes) the engine could SEGFAULT.
Make MSG_WriteBits use an exact buffer overflow check instead of
possibly failing with a few bytes left.

[smcv: Adapt for OpenJK. CVE-2017-11721 has been allocated for the
read buffer overflow.]

Signed-off-by: Simon McVittie <smcv@debian.org>
Co-authored-by: Zack Middleton <zack@cloemail.com>
  • Loading branch information
@smcv @zturtleman
smcv and zturtleman committed Oct 3, 2023
1 parent be28b4e commit bf403f7
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 17 deletions.
27 changes: 18 additions & 9 deletions codemp/qcommon/huffman.cpp
View file
Expand Up @@ -269,9 +269,14 @@ int Huff_Receive (node_t *node, int *ch, byte *fin) {
}

/* Get a symbol */
void Huff_offsetReceive (node_t *node, int *ch, byte *fin, int *offset) {
void Huff_offsetReceive (node_t *node, int *ch, byte *fin, int *offset, int maxoffset) {
bloc = *offset;
while (node && node->symbol == INTERNAL_NODE) {
if (bloc >= maxoffset) {
*ch = 0;
*offset = maxoffset + 1;
return;
}
if (get_bit(fin)) {
node = node->right;
} else {
Expand All @@ -288,11 +293,15 @@ void Huff_offsetReceive (node_t *node, int *ch, byte *fin, int *offset) {
}

/* Send the prefix code for this node */
static void send(node_t *node, node_t *child, byte *fout) {
static void send(node_t *node, node_t *child, byte *fout, int maxoffset) {
if (node->parent) {
send(node->parent, node, fout);
send(node->parent, node, fout, maxoffset);
}
if (child) {
if (bloc >= maxoffset) {
bloc = maxoffset + 1;
return;
}
if (node->right == child) {
add_bit(1, fout);
} else {
Expand All @@ -302,22 +311,22 @@ static void send(node_t *node, node_t *child, byte *fout) {
}

/* Send a symbol */
void Huff_transmit (huff_t *huff, int ch, byte *fout) {
void Huff_transmit (huff_t *huff, int ch, byte *fout, int maxoffset) {
int i;
if (huff->loc[ch] == NULL) {
/* node_t hasn't been transmitted, send a NYT, then the symbol */
Huff_transmit(huff, NYT, fout);
Huff_transmit(huff, NYT, fout, maxoffset);
for (i = 7; i >= 0; i--) {
add_bit((char)((ch >> i) & 0x1), fout);
}
} else {
send(huff->loc[ch], NULL, fout);
send(huff->loc[ch], NULL, fout, maxoffset);
}
}

void Huff_offsetTransmit (huff_t *huff, int ch, byte *fout, int *offset) {
void Huff_offsetTransmit (huff_t *huff, int ch, byte *fout, int *offset, int maxoffset) {
bloc = *offset;
send(huff->loc[ch], NULL, fout);
send(huff->loc[ch], NULL, fout, maxoffset);
*offset = bloc;
}

Expand Down Expand Up @@ -404,7 +413,7 @@ void Huff_Compress(msg_t *mbuf, int offset) {

for (i=0; i<size; i++ ) {
ch = buffer[i];
Huff_transmit(&huff, ch, seq); /* Transmit symbol */
Huff_transmit(&huff, ch, seq, size<<3); /* Transmit symbol */
Huff_addRef(&huff, (byte)ch); /* Do update */
}

Expand Down
41 changes: 36 additions & 5 deletions codemp/qcommon/msg.cpp
View file
Expand Up @@ -139,9 +139,7 @@ void MSG_WriteBits( msg_t *msg, int value, int bits ) {

oldsize += bits;

// this isn't an exact overflow check, but close enough
if ( msg->maxsize - msg->cursize < 4 ) {
msg->overflowed = qtrue;
if ( msg->overflowed ) {
return;
}

Expand Down Expand Up @@ -175,6 +173,11 @@ void MSG_WriteBits( msg_t *msg, int value, int bits ) {
bits = -bits;
}
if (msg->oob) {
if ( msg->cursize + ( bits >> 3 ) > msg->maxsize ) {
msg->overflowed = qtrue;
return;
}

if (bits==8) {
msg->data[msg->cursize] = value;
msg->cursize += 1;
Expand All @@ -197,6 +200,10 @@ void MSG_WriteBits( msg_t *msg, int value, int bits ) {
if (bits&7) {
int nbits;
nbits = bits&7;
if ( msg->bit + nbits > msg->maxsize << 3 ) {
msg->overflowed = qtrue;
return;
}
for(i=0;i<nbits;i++) {
Huff_putBit((value&1), msg->data, &msg->bit);
value = (value>>1);
Expand All @@ -208,8 +215,13 @@ void MSG_WriteBits( msg_t *msg, int value, int bits ) {
#ifdef _NEWHUFFTABLE_
fwrite(&value, 1, 1, fp);
#endif // _NEWHUFFTABLE_
Huff_offsetTransmit (&msgHuff.compressor, (value&0xff), msg->data, &msg->bit);
Huff_offsetTransmit (&msgHuff.compressor, (value&0xff), msg->data, &msg->bit, msg->maxsize << 3);
value = (value>>8);

if ( msg->bit > msg->maxsize << 3 ) {
msg->overflowed = qtrue;
return;
}
}
}
msg->cursize = (msg->bit>>3)+1;
Expand All @@ -221,6 +233,11 @@ int MSG_ReadBits( msg_t *msg, int bits ) {
int get;
qboolean sgn;
int i, nbits;

if ( msg->readcount > msg->cursize ) {
return 0;
}

value = 0;

if ( bits < 0 ) {
Expand All @@ -231,6 +248,11 @@ int MSG_ReadBits( msg_t *msg, int bits ) {
}

if (msg->oob) {
if (msg->readcount + (bits>>3) > msg->cursize) {
msg->readcount = msg->cursize + 1;
return 0;
}

if (bits==8) {
value = msg->data[msg->readcount];
msg->readcount += 1;
Expand All @@ -253,18 +275,27 @@ int MSG_ReadBits( msg_t *msg, int bits ) {
nbits = 0;
if (bits&7) {
nbits = bits&7;
if (msg->bit + nbits > msg->cursize << 3) {
msg->readcount = msg->cursize + 1;
return 0;
}
for(i=0;i<nbits;i++) {
value |= (Huff_getBit(msg->data, &msg->bit)<<i);
}
bits = bits - nbits;
}
if (bits) {
for(i=0;i<bits;i+=8) {
Huff_offsetReceive (msgHuff.decompressor.tree, &get, msg->data, &msg->bit);
Huff_offsetReceive (msgHuff.decompressor.tree, &get, msg->data, &msg->bit, msg->cursize<<3);
#ifdef _NEWHUFFTABLE_
fwrite(&get, 1, 1, fp);
#endif // _NEWHUFFTABLE_
value |= (get<<(i+nbits));

if (msg->bit > msg->cursize<<3) {
msg->readcount = msg->cursize + 1;
return 0;
}
}
}
msg->readcount = (msg->bit>>3)+1;
Expand Down
6 changes: 3 additions & 3 deletions codemp/qcommon/qcommon.h
View file
Expand Up @@ -1034,9 +1034,9 @@ void Huff_Decompress(msg_t *buf, int offset);
void Huff_Init(huffman_t *huff);
void Huff_addRef(huff_t* huff, byte ch);
int Huff_Receive (node_t *node, int *ch, byte *fin);
void Huff_transmit (huff_t *huff, int ch, byte *fout);
void Huff_offsetReceive (node_t *node, int *ch, byte *fin, int *offset);
void Huff_offsetTransmit (huff_t *huff, int ch, byte *fout, int *offset);
void Huff_transmit (huff_t *huff, int ch, byte *fout, int maxoffset);
void Huff_offsetReceive (node_t *node, int *ch, byte *fin, int *offset, int maxoffset);
void Huff_offsetTransmit (huff_t *huff, int ch, byte *fout, int *offset, int maxoffset);
void Huff_putBit( int bit, byte *fout, int *offset);
int Huff_getBit( byte *fout, int *offset);

Expand Down

0 comments on commit bf403f7

Please sign in to comment.