[ioquake3] 10/21: Fix/improve buffer overflow in MSG_ReadBits/MSG_WriteBits

Simon McVittie smcv at debian.org
Fri Aug 4 20:39:13 UTC 2017


This is an automated email from the git hooks/post-receive script.

smcv pushed a commit to branch debian/master
in repository ioquake3.

commit d2b1d124d4055c2fcbe5126863487c52fd58cca1
Author: Zack Middleton <zack at cloemail.com>
Date:   Wed Aug 2 14:55:10 2017 -0500

    Fix/improve buffer overflow in MSG_ReadBits/MSG_WriteBits
    
    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.
---
 code/qcommon/huffman.c | 27 ++++++++++++++++++---------
 code/qcommon/msg.c     | 40 +++++++++++++++++++++++++++++++++++-----
 code/qcommon/qcommon.h |  6 +++---
 3 files changed, 56 insertions(+), 17 deletions(-)

diff --git a/code/qcommon/huffman.c b/code/qcommon/huffman.c
index c1b9f24..1f42498 100644
--- a/code/qcommon/huffman.c
+++ b/code/qcommon/huffman.c
@@ -279,9 +279,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 {
@@ -298,11 +303,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 {
@@ -312,22 +321,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;
 }
 
@@ -413,7 +422,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 */
 	}
 
diff --git a/code/qcommon/msg.c b/code/qcommon/msg.c
index 20dec91..eb767be 100644
--- a/code/qcommon/msg.c
+++ b/code/qcommon/msg.c
@@ -107,9 +107,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;
 	}
 
@@ -122,6 +120,11 @@ void MSG_WriteBits( msg_t *msg, int value, int 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;
@@ -144,6 +147,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);
@@ -152,8 +159,13 @@ void MSG_WriteBits( msg_t *msg, int value, int bits ) {
 		}
 		if ( bits ) {
 			for( i = 0; i < bits; i += 8 ) {
-				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;
@@ -167,6 +179,10 @@ int MSG_ReadBits( msg_t *msg, int bits ) {
 	int			i, nbits;
 //	FILE*	fp;
 
+	if ( msg->readcount > msg->cursize ) {
+		return 0;
+	}
+
 	value = 0;
 
 	if ( bits < 0 ) {
@@ -177,6 +193,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];
@@ -204,6 +225,10 @@ 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);
 			}
@@ -212,9 +237,14 @@ int MSG_ReadBits( msg_t *msg, int bits ) {
 		if (bits) {
 //			fp = fopen("c:\\netchan.bin", "a");
 			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);
 //				fwrite(&get, 1, 1, fp);
 				value |= (get<<(i+nbits));
+
+				if (msg->bit > msg->cursize<<3) {
+					msg->readcount = msg->cursize + 1;
+					return 0;
+				}
 			}
 //			fclose(fp);
 		}
diff --git a/code/qcommon/qcommon.h b/code/qcommon/qcommon.h
index 124c393..07eebe7 100644
--- a/code/qcommon/qcommon.h
+++ b/code/qcommon/qcommon.h
@@ -1196,9 +1196,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);
 

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-games/ioquake3.git



More information about the Pkg-games-commits mailing list