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

CVE-2017-11721: buffer overflow in MSG_ReadBits #937

Merged
merged 1 commit into from
Oct 3, 2023

Conversation

smcv
Copy link
Contributor

@smcv smcv commented Aug 5, 2017

@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:

  • When reading data out of a message, a crafted message can arrange for the engine (client or server or both, I'm not sure) to read beyond the message bounds, potentially causing a crash (denial of service) or disclosure of memory contents. Remotely crashing a client or server is a minor security vulnerability (denial of service) so CVE-2017-11721 was assigned.
  • When writing data into a message, there is no vulnerability, but the checks for overflow are too pessimistic and can report that a write would overflow when in fact there are a few bytes still available for writing. @zturtleman fixed this too, in the same commit.

(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.

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>
@ensiform
Copy link
Member

ensiform commented Aug 5, 2017

OpenJK does not have the VoIP system where the issue permeates though, unless it can suddenly be abused elsewhere?

@smcv
Copy link
Contributor Author

smcv commented Aug 6, 2017

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.

@ensiform
Copy link
Member

ensiform commented Aug 6, 2017

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.

@smcv
Copy link
Contributor Author

smcv commented Aug 9, 2017

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.

@ensiform
Copy link
Member

ensiform commented Aug 9, 2017

I would rather this fix instead tbh

ec-/Quake3e@3743137

@Razish
Copy link
Member

Razish commented Sep 25, 2023

Reviving this ancient PR; I don't really care which fix gets merged. Let's just merge this PR?
I suspect q3e's has other dependent commits that don't match OJK.
Since it has a CVE against it, I'm gonna merge this if there's no alternative in a week. It's already 6 years overdue.

@Razish Razish merged commit bf403f7 into JACoders:master Oct 3, 2023
MaceMadunusus pushed a commit to MBII/OpenJK that referenced this pull request Oct 24, 2023
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>
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.

4 participants