-
Notifications
You must be signed in to change notification settings - Fork 614
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
CVE-2017-11721: buffer overflow in MSG_ReadBits #937
Conversation
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. [smcv: Adapt for OpenJK. CVE-2017-11721 has been allocated for the read buffer overflow.] Signed-off-by: Simon McVittie <smcv@debian.org>
OpenJK does not have the VoIP system where the issue permeates though, unless it can suddenly be abused elsewhere? |
Sorry, I have no idea what this Huffman compression is used for - it might only be VoIP (and hence dead code in OpenJK) or it might be used for normal gameplay. |
Huffman is used for most message transmission, but that does not mean that it was susceptible outside of those parameters. AFAIK the original report was specific to a specially crafted voip packet in ioquake3. If you believe its still vulnerable in general, then maybe its fine. |
If the overflow is only exploitable via VoIP in practice, then it doesn't need to be treated as a vulnerability, but I think it would still be worthwhile to fix it (so that OpenJK won't get that vulnerability if it later gains VoIP, or some other feature that uses exploitable Huffman-coding parameters). This is less of a concern for me than the equivalents in ioquake3 and iortcw, because OpenJK hasn't had any releases yet, and isn't in a Debian release or the branch that will eventually become the next Debian release. |
I would rather this fix instead tbh |
Reviving this ancient PR; I don't really care which fix gets merged. Let's just merge this PR? |
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. [smcv: Adapt for OpenJK. CVE-2017-11721 has been allocated for the read buffer overflow.] Signed-off-by: Simon McVittie <smcv@debian.org> Co-authored-by: Zack Middleton <zack@cloemail.com>
@zturtleman recently committed a fix to the Huffman coding in ioquake3's netcode (ioquake/ioq3@d2b1d12). This has been treated as a security vulnerability and CVE-2017-11721 was assigned (by someone, it isn't clear who requested this): see https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-11721. As another descendant of the Quake III Arena engine, OpenJK seems to have the same issue.
I believe the situation is:
(But my interpretation might be wrong, I haven't studied this in detail.)
I have ported the ioquake3 patch to OpenJK's netcode. The result seems to work in some very brief testing, so I have uploaded it to Debian. Please consider merging.