out of bounds memory access in snd_mix.cpp S_PaintChannelFrom16

[namtsui]

Reporting a bug? Please make sure you've given the following information - thanks!

Operating system and version:

OpenBSD amd64
5203023

Is this for single player or multiplayer?

multiplayer

Description of the bug (and if possible, steps to reproduce the bug):

In multiplayer I created a game on Vjun Sentinel, got the Merr-Sonn PLX-2M behind the lifts near the starting spawn at lifts, and segfaulted either immediately or several seconds after firing it. This is somewhat hard to reproduce as most of the time I am able to fire without crashing.

(gdb) run
Starting program: /usr/local/share/JediAcademy/openjk 
[New thread 614481]
Thread 1 received signal SIGSEGV, Segmentation fault.
0x000006dce7350b97 in S_PaintChannelFrom16 (ch=0x6dce8177d00 <s_channels+920928>, sfx=0x6dce82f8890 <s_knownSfx+40832>, count=1024, sampleOffset=133859, bufferOffset=0) at /usr/ports/pobj/openjk-0.0.0.20200214/OpenJK-52030235f052772008d99e6ccb16de48e7ddb688/codemp/client/snd_mix.cpp:284
284			iData = sfx->pSoundData[ (int)ofst ];
(gdb) bt
#0  0x000006dce7350b97 in S_PaintChannelFrom16 (ch=0x6dce8177d00 <s_channels+920928>, sfx=0x6dce82f8890 <s_knownSfx+40832>, count=1024, sampleOffset=133859, bufferOffset=0) at /usr/ports/pobj/openjk-0.0.0.20200214/OpenJK-52030235f052772008d99e6ccb16de48e7ddb688/codemp/client/snd_mix.cpp:284
#1  0x000006dce7350ac9 in ChannelPaint (ch=0x6dce8177d00 <s_channels+920928>, sc=0x6dce82f8890 <s_knownSfx+40832>, count=1024, sampleOffset=133859, bufferOffset=0) at /usr/ports/pobj/openjk-0.0.0.20200214/OpenJK-52030235f052772008d99e6ccb16de48e7ddb688/codemp/client/snd_mix.cpp:353
#2  0x000006dce7350f7c in S_PaintChannels (endtime=849524) at /usr/ports/pobj/openjk-0.0.0.20200214/OpenJK-52030235f052772008d99e6ccb16de48e7ddb688/codemp/client/snd_mix.cpp:451
#3  0x000006dce734b396 in S_Update_ () at /usr/ports/pobj/openjk-0.0.0.20200214/OpenJK-52030235f052772008d99e6ccb16de48e7ddb688/codemp/client/snd_dma.cpp:3118
#4  0x000006dce734aec6 in S_Update () at /usr/ports/pobj/openjk-0.0.0.20200214/OpenJK-52030235f052772008d99e6ccb16de48e7ddb688/codemp/client/snd_dma.cpp:2788
#5  0x000006dce730f020 in CL_Frame (msec=8) at /usr/ports/pobj/openjk-0.0.0.20200214/OpenJK-52030235f052772008d99e6ccb16de48e7ddb688/codemp/client/cl_main.cpp:2209
#6  0x000006dce722ec7a in Com_Frame () at /usr/ports/pobj/openjk-0.0.0.20200214/OpenJK-52030235f052772008d99e6ccb16de48e7ddb688/codemp/qcommon/common.cpp:1585
#7  0x000006dce73852b7 in main (argc=1, argv=0x7f7ffffe69b8) at /usr/ports/pobj/openjk-0.0.0.20200214/OpenJK-52030235f052772008d99e6ccb16de48e7ddb688/shared/sys/sys_main.cpp:812
(gdb) print ofst
$1 = 143352
(gdb) print iData
$2 = 0
(gdb) print sfx->pSoundData
$3 = (short *) 0x6df55a1b020
(gdb) print sfx->pSoundData[0]
$4 = -33
(gdb) print sfx->pSoundData[1]
$5 = -33
(gdb) print sfx->pSoundData[2]
$6 = -566
(gdb) print sfx->pSoundData[3]
$7 = -566
(gdb) print sfx->pSoundData[1000]
$8 = -2543
(gdb) print sfx->pSoundData[ofst]
Cannot access memory at address 0x6df55a61010
(gdb) print sfx->pSoundData[ofst-1]
Cannot access memory at address 0x6df55a6100e
(gdb) print sfx->pSoundData[ofst-2]
Cannot access memory at address 0x6df55a6100c
(gdb) print sfx->pSoundData[ofst-1000]
$9 = 0
(gdb) print sfx->pSoundData[ofst-2000]
$10 = 3519

What did you expect to happen instead?

It looks like ofst is causing out of bounds memory access.

[namtsui]

I saw that iSoundLengthInSamples is used as a bounds check.

OpenJK/codemp/client/snd_dma.cpp

Lines 2412 to 2416 in 5203023

	// have we run off the end?
	if ((offset + (i*100)) > ch->thesfx->iSoundLengthInSamples)
	{
	break;
	}

These lines were useful to understand that iSoundLengthInSamples is number of samples stored as shorts. pSoundData is a buffer with shorts, each 2 bytes, so malloc iSoundLengthOfSamples * 2 bytes.

OpenJK/codemp/client/snd_dma.cpp

Lines 885 to 886 in 5203023

	sfx->iSoundLengthInSamples = 512; // #samples, ie shorts
	sfx->pSoundData = (short *) SND_malloc(512*2, sfx); // ... so *2 for alloc bytes

I tested with `sysctl vm.malloc_conf=S' on OpenBSD, which enables various security auditing options for malloc in the hopes of triggering this crash more often. It remains somewhat hard to reproduce even with this malloc option.

This adds a bounds check to prevent buffer over-reading. With this fix I tested loading into the map and shooting the Merr-Sonn PLX-2M. It no longer crashes.

Related commit: 69800e8

[namtsui]

gdb showing that ofst is somewhat close to sfx->iSoundLengthInSamples at the time of the segfault.

(gdb) run
Starting program: /usr/local/share/JediAcademy/openjk 
[New thread 382056]

Thread 1 received signal SIGSEGV, Segmentation fault.
0x0000092ad753db97 in S_PaintChannelFrom16 (ch=0x92ad8307218 <s_channels+537208>, sfx=0x92ad84e5890 <s_knownSfx+40832>, count=1024, sampleOffset=135381, bufferOffset=0) at /usr/ports/pobj/openjk-0.0.0.20200214/OpenJK-52030235f052772008d99e6ccb16de48e7ddb688/codemp/client/snd_mix.cpp:284
284			iData = sfx->pSoundData[ (int)ofst ];
(gdb) print ofst
$1 = 143350.078
(gdb) print sfx->iSoundLengthInSamples
$2 = 141810
(gdb) print sfx->pSoundData[141810]
$3 = 25991
(gdb) print sfx->pSoundData[141811]
$4 = 8515
(gdb) print sfx->pSoundData[141812]
$5 = -9253
(gdb) print sfx->pSoundData[141813]
$6 = -9253
(gdb) print sfx->pSoundData[141814]
$7 = -9253
(gdb) print sfx->pSoundData[141815]
$8 = -9253
(gdb) print sfx->pSoundData[141820]
$9 = -9253
(gdb) print sfx->pSoundData[1418]
$10 = 7229
(gdb) print sfx->pSoundData[1418]
$11 = 7229
(gdb) print sfx->pSoundData[ofst]
Cannot access memory at address 0x92d08c6c00c
(gdb) print sfx->pSoundData[ofst]
Cannot access memory at address 0x92d08c6c00c
(gdb) print sfx->pSoundData[141809]
$12 = 992