Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add bounds check to resolve buffer over-read #1040

Merged
merged 2 commits into from
Jun 9, 2020

Conversation

namtsui
Copy link
Contributor

@namtsui namtsui commented May 21, 2020

Fixes #1038

ofst over-reads past the end of the pSoundData buffer. Add a bounds check
similar to S_CheckAmplitude() in codemp/client/snd_dma.cpp.

69800e8

Fixes JACoders#1038

ofst over-reads past the end of the pSoundData buffer. Add a bounds check
similar to S_CheckAmplitude() in codemp/client/snd_dma.cpp.

JACoders@69800e8
@xycaleth
Copy link
Member

Thanks for the PR! I think a better place to fix this is here:
https://github.com/JACoders/OpenJK/blob/master/codemp/client/snd_mix.cpp#L445-L448

Before the code calls S_PaintChannelFrom16 (from ChannelPaint)

Then we have the correct number of samples to add from the start and don't need to perform a comparison every sample loop

Sounds that use dopplerScale (e.g., rocket launcher) exhibited a buffer
over-read. S_PaintChannelFrom16's ofst reads past end of sfx->pSoundData buffer.
To resolve this, take dopplerScale increments of ofst into consideration when
calculating count, which controls the loop for ofst.

Resolves: JACoders#1038
See also: JACoders@69800e8
@namtsui
Copy link
Contributor Author

namtsui commented May 22, 2020

That seems like a better place to put it. Please test this to make sure I did not break the doppler effect when shooting a rocket launcher. It works in my testing with ten runs of shooting the rocket launcher.

@xycaleth
Copy link
Member

xycaleth commented Jun 9, 2020

I'm unable to test right now but the code looks correct and I'll trust your testing went fine :)

@xycaleth xycaleth merged commit 0218d2d into JACoders:master Jun 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

out of bounds memory access in snd_mix.cpp S_PaintChannelFrom16
2 participants