Skip to content

Commit d2b1d12

Browse files
committed
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.
1 parent 2d6171f commit d2b1d12

File tree

3 files changed

+56
-17
lines changed

3 files changed

+56
-17
lines changed

Diff for: code/qcommon/huffman.c

+18-9
Original file line numberDiff line numberDiff line change
@@ -279,9 +279,14 @@ int Huff_Receive (node_t *node, int *ch, byte *fin) {
279279
}
280280

281281
/* Get a symbol */
282-
void Huff_offsetReceive (node_t *node, int *ch, byte *fin, int *offset) {
282+
void Huff_offsetReceive (node_t *node, int *ch, byte *fin, int *offset, int maxoffset) {
283283
bloc = *offset;
284284
while (node && node->symbol == INTERNAL_NODE) {
285+
if (bloc >= maxoffset) {
286+
*ch = 0;
287+
*offset = maxoffset + 1;
288+
return;
289+
}
285290
if (get_bit(fin)) {
286291
node = node->right;
287292
} else {
@@ -298,11 +303,15 @@ void Huff_offsetReceive (node_t *node, int *ch, byte *fin, int *offset) {
298303
}
299304

300305
/* Send the prefix code for this node */
301-
static void send(node_t *node, node_t *child, byte *fout) {
306+
static void send(node_t *node, node_t *child, byte *fout, int maxoffset) {
302307
if (node->parent) {
303-
send(node->parent, node, fout);
308+
send(node->parent, node, fout, maxoffset);
304309
}
305310
if (child) {
311+
if (bloc >= maxoffset) {
312+
bloc = maxoffset + 1;
313+
return;
314+
}
306315
if (node->right == child) {
307316
add_bit(1, fout);
308317
} else {
@@ -312,22 +321,22 @@ static void send(node_t *node, node_t *child, byte *fout) {
312321
}
313322

314323
/* Send a symbol */
315-
void Huff_transmit (huff_t *huff, int ch, byte *fout) {
324+
void Huff_transmit (huff_t *huff, int ch, byte *fout, int maxoffset) {
316325
int i;
317326
if (huff->loc[ch] == NULL) {
318327
/* node_t hasn't been transmitted, send a NYT, then the symbol */
319-
Huff_transmit(huff, NYT, fout);
328+
Huff_transmit(huff, NYT, fout, maxoffset);
320329
for (i = 7; i >= 0; i--) {
321330
add_bit((char)((ch >> i) & 0x1), fout);
322331
}
323332
} else {
324-
send(huff->loc[ch], NULL, fout);
333+
send(huff->loc[ch], NULL, fout, maxoffset);
325334
}
326335
}
327336

328-
void Huff_offsetTransmit (huff_t *huff, int ch, byte *fout, int *offset) {
337+
void Huff_offsetTransmit (huff_t *huff, int ch, byte *fout, int *offset, int maxoffset) {
329338
bloc = *offset;
330-
send(huff->loc[ch], NULL, fout);
339+
send(huff->loc[ch], NULL, fout, maxoffset);
331340
*offset = bloc;
332341
}
333342

@@ -413,7 +422,7 @@ void Huff_Compress(msg_t *mbuf, int offset) {
413422

414423
for (i=0; i<size; i++ ) {
415424
ch = buffer[i];
416-
Huff_transmit(&huff, ch, seq); /* Transmit symbol */
425+
Huff_transmit(&huff, ch, seq, size<<3); /* Transmit symbol */
417426
Huff_addRef(&huff, (byte)ch); /* Do update */
418427
}
419428

Diff for: code/qcommon/msg.c

+35-5
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,7 @@ void MSG_WriteBits( msg_t *msg, int value, int bits ) {
107107

108108
oldsize += bits;
109109

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

@@ -122,6 +120,11 @@ void MSG_WriteBits( msg_t *msg, int value, int bits ) {
122120
}
123121

124122
if ( msg->oob ) {
123+
if ( msg->cursize + ( bits >> 3 ) > msg->maxsize ) {
124+
msg->overflowed = qtrue;
125+
return;
126+
}
127+
125128
if ( bits == 8 ) {
126129
msg->data[msg->cursize] = value;
127130
msg->cursize += 1;
@@ -144,6 +147,10 @@ void MSG_WriteBits( msg_t *msg, int value, int bits ) {
144147
if ( bits&7 ) {
145148
int nbits;
146149
nbits = bits&7;
150+
if ( msg->bit + nbits > msg->maxsize << 3 ) {
151+
msg->overflowed = qtrue;
152+
return;
153+
}
147154
for( i = 0; i < nbits; i++ ) {
148155
Huff_putBit( (value & 1), msg->data, &msg->bit );
149156
value = (value >> 1);
@@ -152,8 +159,13 @@ void MSG_WriteBits( msg_t *msg, int value, int bits ) {
152159
}
153160
if ( bits ) {
154161
for( i = 0; i < bits; i += 8 ) {
155-
Huff_offsetTransmit( &msgHuff.compressor, (value & 0xff), msg->data, &msg->bit );
162+
Huff_offsetTransmit( &msgHuff.compressor, (value & 0xff), msg->data, &msg->bit, msg->maxsize << 3 );
156163
value = (value >> 8);
164+
165+
if ( msg->bit > msg->maxsize << 3 ) {
166+
msg->overflowed = qtrue;
167+
return;
168+
}
157169
}
158170
}
159171
msg->cursize = (msg->bit >> 3) + 1;
@@ -167,6 +179,10 @@ int MSG_ReadBits( msg_t *msg, int bits ) {
167179
int i, nbits;
168180
// FILE* fp;
169181

182+
if ( msg->readcount > msg->cursize ) {
183+
return 0;
184+
}
185+
170186
value = 0;
171187

172188
if ( bits < 0 ) {
@@ -177,6 +193,11 @@ int MSG_ReadBits( msg_t *msg, int bits ) {
177193
}
178194

179195
if (msg->oob) {
196+
if (msg->readcount + (bits>>3) > msg->cursize) {
197+
msg->readcount = msg->cursize + 1;
198+
return 0;
199+
}
200+
180201
if(bits==8)
181202
{
182203
value = msg->data[msg->readcount];
@@ -204,6 +225,10 @@ int MSG_ReadBits( msg_t *msg, int bits ) {
204225
nbits = 0;
205226
if (bits&7) {
206227
nbits = bits&7;
228+
if (msg->bit + nbits > msg->cursize << 3) {
229+
msg->readcount = msg->cursize + 1;
230+
return 0;
231+
}
207232
for(i=0;i<nbits;i++) {
208233
value |= (Huff_getBit(msg->data, &msg->bit)<<i);
209234
}
@@ -212,9 +237,14 @@ int MSG_ReadBits( msg_t *msg, int bits ) {
212237
if (bits) {
213238
// fp = fopen("c:\\netchan.bin", "a");
214239
for(i=0;i<bits;i+=8) {
215-
Huff_offsetReceive (msgHuff.decompressor.tree, &get, msg->data, &msg->bit);
240+
Huff_offsetReceive (msgHuff.decompressor.tree, &get, msg->data, &msg->bit, msg->cursize<<3);
216241
// fwrite(&get, 1, 1, fp);
217242
value |= (get<<(i+nbits));
243+
244+
if (msg->bit > msg->cursize<<3) {
245+
msg->readcount = msg->cursize + 1;
246+
return 0;
247+
}
218248
}
219249
// fclose(fp);
220250
}

Diff for: code/qcommon/qcommon.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -1196,9 +1196,9 @@ void Huff_Decompress(msg_t *buf, int offset);
11961196
void Huff_Init(huffman_t *huff);
11971197
void Huff_addRef(huff_t* huff, byte ch);
11981198
int Huff_Receive (node_t *node, int *ch, byte *fin);
1199-
void Huff_transmit (huff_t *huff, int ch, byte *fout);
1200-
void Huff_offsetReceive (node_t *node, int *ch, byte *fin, int *offset);
1201-
void Huff_offsetTransmit (huff_t *huff, int ch, byte *fout, int *offset);
1199+
void Huff_transmit (huff_t *huff, int ch, byte *fout, int maxoffset);
1200+
void Huff_offsetReceive (node_t *node, int *ch, byte *fin, int *offset, int maxoffset);
1201+
void Huff_offsetTransmit (huff_t *huff, int ch, byte *fout, int *offset, int maxoffset);
12021202
void Huff_putBit( int bit, byte *fout, int *offset);
12031203
int Huff_getBit( byte *fout, int *offset);
12041204

0 commit comments

Comments
 (0)