Reading outside a buffer while calculating checksum
Brought to you by:
e6y
Menu
▾
▴
#251 Reading outside a buffer while calculating checksum
I_GetPacket incorrectly passes length variable to ChecksumPacket. In the result, reading outisde a buffer because is performed. The crash happens in the client while connecting to the server.
Tested on: Ubuntu 16.04 32 bit
Code from: https://svn.prboom.org/repos/branches/prboom-plus-24/prboom2/
Game data: FreeDM from https://freedoom.github.io/download.html
Compilation with asan:
root@ubuntu16:~/projects/prboom-plus/prboom2# sed -i 's/^CFLAGS_OPT=.*/CFLAGS_OPT="-O0 -ggdb3"/' configure.ac
root@ubuntu16:~/projects/prboom-plus/prboom2# ./bootstrap
root@ubuntu16:~/projects/prboom-plus/prboom2# CFLAGS="-fsanitize=address" LDFLAGS="-fsanitize=address" ./configure --prefix=`pwd`/bin
root@ubuntu16:~/projects/prboom-plus/prboom2# make install
Starting the server:
root@ubuntu16:~/projects/prboom-plus/prboom2/bin/games# ./prboom-plus-game-server
Listening on port 5030, waiting for 2 players
Starting the client with crash:
root@ubuntu16:~/projects/prboom-plus/prboom2/bin/games# ./prboom-plus -iwad /root/projects/prboom-plus/freedm-0.11.3/freedm.wad -net 127.0.0.1 -window -nomouse
M_LoadDefaults: Load system defaults.
default file: /root/.prboom-plus/prboom-plus.cfg
found /root/projects/prboom-plus/prboom2/bin/share/games/doom/prboom-plus.wad
PrBoom-Plus v2.5.1.5 (http://prboom-plus.sourceforge.net/)
I_SetAffinityMask: manual affinity mask is 1
found /root/projects/prboom-plus/freedm-0.11.3/freedm.wad
IWAD found: /root/projects/prboom-plus/freedm-0.11.3/freedm.wad
PrBoom-Plus (built 2019-08-17 13:35:34), playing: DOOM 2: Hell on Earth
PrBoom-Plus is released under the GNU General Public license v2.0.
You are welcome to redistribute it under certain conditions.
It comes with ABSOLUTELY NO WARRANTY. See the file COPYING for details.
V_Init: allocate screens.
V_InitMode: using 8 bit video mode
I_CalculateRes: trying to optimize screen pitch
test case for pitch=640 is processed 14185 times for 100 msec
test case for pitch=672 is processed 15243 times for 100 msec
optimized screen pitch is 672
I_InitScreenResolution: Using resolution 640x480
found /root/projects/prboom-plus/prboom2/bin/share/games/doom/prboom-plus.wad
D_InitNetGame: Checking for network game.
joined game as player 1/2; 0 WADs specified
W_Init: Init WADfiles.
adding /root/projects/prboom-plus/freedm-0.11.3/freedm.wad
adding /root/projects/prboom-plus/prboom2/bin/share/games/doom/prboom-plus.wad
W_InitCache
Loading DEH lump from /root/projects/prboom-plus/freedm-0.11.3/freedm.wad
M_Init: Init miscellaneous info.
SetRatio: width/height parameters 640x480
SetRatio: storage aspect ratio 4:3
SetRatio: assuming square pixels
SetRatio: display aspect ratio 4:3
SetRatio: gl_ratio 1.600000
SetRatio: multiplier 1/1
D_CheckNetGame: waiting for server to signal game start
=================================================================
==5322==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xb3822260 at pc 0x0832048a bp 0xbf8408e8 sp 0xbf8408d8
READ of size 1 at 0xb3822260 thread T0
#0 0x8320489 in ChecksumPacket /root/projects/prboom-plus/prboom2/src/SDL/i_network.c:221
#1 0x83206c7 in I_GetPacket /root/projects/prboom-plus/prboom2/src/SDL/i_network.c:243
#2 0x82c26b1 in D_CheckNetGame /root/projects/prboom-plus/prboom2/src/d_client.c:192
#3 0x817da59 in D_DoomMainSetup /root/projects/prboom-plus/prboom2/src/d_main.c:1853
#4 0x817e94b in D_DoomMain /root/projects/prboom-plus/prboom2/src/d_main.c:1977
#5 0x8320041 in main /root/projects/prboom-plus/prboom2/src/SDL/i_main.c:576
#6 0xb73fa636 in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x18636)
#7 0x804daf6 (/root/projects/prboom-plus/prboom2/bin/games/prboom-plus+0x804daf6)
0xb3822260 is located 0 bytes to the right of 64-byte region [0xb3822220,0xb3822260)
allocated by thread T0 here:
#0 0xb7a80b44 in __interceptor_malloc (/usr/lib/i386-linux-gnu/libasan.so.3+0xc3b44)
#1 0x82c0345 in Z_Malloc /root/projects/prboom-plus/prboom2/src/z_zone.c:397
#2 0x82c25d0 in D_CheckNetGame /root/projects/prboom-plus/prboom2/src/d_client.c:187
#3 0x817da59 in D_DoomMainSetup /root/projects/prboom-plus/prboom2/src/d_main.c:1853
#4 0x817e94b in D_DoomMain /root/projects/prboom-plus/prboom2/src/d_main.c:1977
#5 0x8320041 in main /root/projects/prboom-plus/prboom2/src/SDL/i_main.c:576
#6 0xb73fa636 in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x18636)
SUMMARY: AddressSanitizer: heap-buffer-overflow /root/projects/prboom-plus/prboom2/src/SDL/i_network.c:221 in ChecksumPacket
Shadow bytes around the buggy address:
0x367043f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x36704400: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x36704410: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x36704420: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
0x36704430: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x36704440: fa fa fa fa 00 00 00 00 00 00 00 00[fa]fa fa fa
0x36704450: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
0x36704460: 00 00 00 00 fa fa fa fa 00 00 00 00 00 00 00 00
0x36704470: fa fa fa fa 00 00 00 00 00 00 00 00 fa fa fa fa
0x36704480: 00 00 00 00 00 00 00 00 fa fa fa fa 00 00 00 00
0x36704490: 00 00 00 00 fa fa fa fa 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Heap right redzone: fb
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack partial redzone: f4
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
==5322==ABORTING
Function I_GetPacket passes udp_packet->len toCheckSum, but a correct (smaller) buffer's size is stored in len.
226 size_t I_GetPacket(packet_header_t* buffer, size_t buflen)
227 {
228 int checksum;
229 size_t len;
230 int status;
231
232 status = SDLNet_UDP_Recv(udp_socket, udp_packet);
233 len = udp_packet->len;
234 if (buflen<len)
235 len=buflen;
236 if ( (status!=0) && (len>0) )
237 memcpy(buffer, udp_packet->data, len);
(...)
242 if ( (status!=0) && (len>0)) {
243 byte psum = ChecksumPacket(buffer, udp_packet->len);
(...)
249 }
My fix which solved crash problem:
root@ubuntu16:~/projects/prboom-plus/prboom2# svn diff src/SDL/i_network.c
Index: src/SDL/i_network.c
===================================================================
--- src/SDL/i_network.c (revision 4540)
+++ src/SDL/i_network.c (working copy)
@@ -240,7 +240,7 @@
checksum=buffer->checksum;
buffer->checksum=0;
if ( (status!=0) && (len>0)) {
- byte psum = ChecksumPacket(buffer, udp_packet->len);
+ byte psum = ChecksumPacket(buffer, len);
/* fprintf(stderr, "recvlen = %u, stolen = %u, csum = %u, psum = %u\n",
udp_packet->len, len, checksum, psum); */
if (psum == checksum) return len;
