Bug#870725: CVE-2017-11721: read buffer overflow in MSG_ReadBits

2017-08-12 Thread Simon McVittie
On Sat, 12 Aug 2017 at 15:33:34 +0200, Moritz Mühlenhoff wrote:
> Feel free to simply upload an untested package for jessie-security,
> I'm flying back on Sunday evening, I can run tests on jessie sometime
> next week.

I was able to do a smoke-test on a virtual machine (llvmpipe is much
better than I remembered, apparently) so there is now a briefly tested
jessie-security version in the queue. Any additional testing would
likely be useful - the patch is to netcode, so hosting or joining an
openarena server is an appropriate test. See attached debdiff.

S
diffstat for ioquake3-1.36+u20140802+gca9eebb ioquake3-1.36+u20140802+gca9eebb

 changelog|   10 
 patches/security/Fix-improve-buffer-overflow-in-MSG_ReadBits-MSG_WriteBits.patch |  213 ++
 patches/series   |1 
 3 files changed, 224 insertions(+)

diff -Nru ioquake3-1.36+u20140802+gca9eebb/debian/changelog ioquake3-1.36+u20140802+gca9eebb/debian/changelog
--- ioquake3-1.36+u20140802+gca9eebb/debian/changelog	2017-03-14 18:29:48.0 -0400
+++ ioquake3-1.36+u20140802+gca9eebb/debian/changelog	2017-08-12 10:15:49.0 -0400
@@ -1,3 +1,13 @@
+ioquake3 (1.36+u20140802+gca9eebb-2+deb8u2) jessie-security; urgency=medium
+
+  * Add patch from upstream:
++ Address read buffer overflow in
+  MSG_ReadBits (CVE-2017-11721) (Closes: #870725)
++ Check buffer boundary exactly in MSG_WriteBits, instead of
+  potentially failing with a few bytes still available
+
+ -- Simon McVittie   Sat, 12 Aug 2017 10:15:49 -0400
+
 ioquake3 (1.36+u20140802+gca9eebb-2+deb8u1) jessie-security; urgency=high
 
   * d/gbp.conf: switch branch to debian/jessie
diff -Nru ioquake3-1.36+u20140802+gca9eebb/debian/patches/security/Fix-improve-buffer-overflow-in-MSG_ReadBits-MSG_WriteBits.patch ioquake3-1.36+u20140802+gca9eebb/debian/patches/security/Fix-improve-buffer-overflow-in-MSG_ReadBits-MSG_WriteBits.patch
--- ioquake3-1.36+u20140802+gca9eebb/debian/patches/security/Fix-improve-buffer-overflow-in-MSG_ReadBits-MSG_WriteBits.patch	1969-12-31 19:00:00.0 -0500
+++ ioquake3-1.36+u20140802+gca9eebb/debian/patches/security/Fix-improve-buffer-overflow-in-MSG_ReadBits-MSG_WriteBits.patch	2017-08-12 10:15:49.0 -0400
@@ -0,0 +1,213 @@
+From: Zack Middleton 
+Date: Wed, 2 Aug 2017 14:55:10 -0500
+Subject: Fix/improve buffer overflow in MSG_ReadBits/MSG_WriteBits
+
+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.
+
+Origin: upstream, commit:d2b1d124d4055c2fcbe5126863487c52fd58cca1
+Bug-CVE: CVE-2017-11721
+Bug-Debian: https://bugs.debian.org/870725
+---
+ code/qcommon/huffman.c | 27 ++-
+ code/qcommon/msg.c | 40 +++-
+ code/qcommon/qcommon.h |  6 +++---
+ 3 files changed, 56 insertions(+), 17 deletions(-)
+
+diff --git a/code/qcommon/huffman.c b/code/qcommon/huffman.c
+index c1b9f24..1f42498 100644
+--- a/code/qcommon/huffman.c
 b/code/qcommon/huffman.c
+@@ -279,9 +279,14 @@ int Huff_Receive (node_t *node, int *ch, byte *fin) {
+ }
+ 
+ /* Get a symbol */
+-void Huff_offsetReceive (node_t *node, int *ch, byte *fin, int *offset) {
++void Huff_offsetReceive (node_t *node, int *ch, byte *fin, int *offset, int maxoffset) {
+ 	bloc = *offset;
+ 	while (node && node->symbol == INTERNAL_NODE) {
++		if (bloc >= maxoffset) {
++			*ch = 0;
++			*offset = maxoffset + 1;
++			return;
++		}
+ 		if (get_bit(fin)) {
+ 			node = node->right;
+ 		} else {
+@@ -298,11 +303,15 @@ void Huff_offsetReceive (node_t *node, int *ch, byte *fin, int *offset) {
+ }
+ 
+ /* Send the prefix code for this node */
+-static void send(node_t *node, node_t *child, byte *fout) {
++static void send(node_t *node, node_t *child, byte *fout, int maxoffset) {
+ 	if (node->parent) {
+-		send(node->parent, node, fout);
++		send(node->parent, node, fout, maxoffset);
+ 	}
+ 	if (child) {
++		if (bloc >= maxoffset) {
++			bloc = maxoffset + 1;
++			return;
++		}
+ 		if (node->right == child) {
+ 			add_bit(1, fout);
+ 		} else {
+@@ -312,22 +321,22 @@ static void send(node_t *node, node_t *child, byte *fout) {
+ }
+ 
+ /* Send a symbol */
+-void Huff_transmit (huff_t *huff, int ch, byte *fout) {
++void Huff_transmit (huff_t *huff, int ch, byte *fout, int maxoffset) {
+ 	int i;
+ 	if (huff->loc[ch] == NULL) { 
+ 		/* node_t hasn't been transmitted, send a NYT, then the symbol */
+-		Huff_transmit(huff, NYT, fout);
++		Huff_transmit(huff, NYT, fout, maxoffset);
+ 		for (i = 7; i >= 0; i--) {
+ 			add_bit((char)((ch >> i) & 0x1), fout);
+ 		}
+ 	} else {
+-		send(huff->loc[ch], NULL, fout);
++		send(huff->loc[ch], NULL, fout, maxoffset);
+ 	}
+ }
+ 
+-void Huff_offsetTransmit (huff_t *huff, int ch, byte *

Bug#870725: CVE-2017-11721: read buffer overflow in MSG_ReadBits

2017-08-12 Thread Moritz Mühlenhoff
On Sat, Aug 12, 2017 at 01:46:33AM -0400, Simon McVittie wrote:
> On Fri, 11 Aug 2017 at 20:11:46 +0200, Moritz Mühlenhoff wrote:
> > Thanks, please upload. Generally speaking contrib is not supported, but
> > it would be silly to fix ioquake, but not iortcw along, so please go ahead.
> 
> Thanks, both uploaded to security-master targeting stretch-security.
> 
> > What about jessie, is that still usable against current game servers?
> 
> It would make sense to fix ioquake3 in jessie, but I am unlikely to
> be able to complete this work any time soon - I probably won't find a
> jessie user at DebConf, and soon after I get back I'll be moving house,
> so my time and hardware are limited. I'll try to prepare packages so
> that someone else can test them (via openarena).

Feel free to simply upload an untested package for jessie-security,
I'm flying back on Sunday evening, I can run tests on jessie sometime
next week.

Cheers,
Moritz



Bug#870725: CVE-2017-11721: read buffer overflow in MSG_ReadBits

2017-08-11 Thread Simon McVittie
On Fri, 11 Aug 2017 at 20:11:46 +0200, Moritz Mühlenhoff wrote:
> Thanks, please upload. Generally speaking contrib is not supported, but
> it would be silly to fix ioquake, but not iortcw along, so please go ahead.

Thanks, both uploaded to security-master targeting stretch-security.

> What about jessie, is that still usable against current game servers?

It would make sense to fix ioquake3 in jessie, but I am unlikely to
be able to complete this work any time soon - I probably won't find a
jessie user at DebConf, and soon after I get back I'll be moving house,
so my time and hardware are limited. I'll try to prepare packages so
that someone else can test them (via openarena).

For completeness: iortcw isn't in jessie, so not applicable.

S



Bug#870725: CVE-2017-11721: read buffer overflow in MSG_ReadBits

2017-08-11 Thread Moritz Mühlenhoff
On Thu, Aug 10, 2017 at 02:29:52PM -0400, Simon McVittie wrote:
> On Sat, 05 Aug 2017 at 12:24:19 +0100, Simon McVittie wrote:
> > Again, I don't have time to handle this for stable right now, so
> > security or games team members are very welcome to do so. I'll prepare
> > a stable update during Debconf if nobody gets there first, assuming I
> > can find a stable user willing to test a game from contrib.
> 
> I have prepared proposed stable-security updates and borrowed a
> stable machine to smoke-test them (thanks to Andy Simpkins). I forget
> whether you're interested in fixing contrib or not, so I'm doing iortcw
> as well as ioquake3 (openjk is experimental-only so is not relevant here).
> Let me know if I should redirect the iortcw update to the release team.

Thanks, please upload. Generally speaking contrib is not supported, but
it would be silly to fix ioquake, but not iortcw along, so please go ahead.

What about jessie, is that still usable against current game servers?

Cheers,
Moritz



Bug#870725: CVE-2017-11721: read buffer overflow in MSG_ReadBits

2017-08-10 Thread Simon McVittie
On Sat, 05 Aug 2017 at 12:24:19 +0100, Simon McVittie wrote:
> Again, I don't have time to handle this for stable right now, so
> security or games team members are very welcome to do so. I'll prepare
> a stable update during Debconf if nobody gets there first, assuming I
> can find a stable user willing to test a game from contrib.

I have prepared proposed stable-security updates and borrowed a
stable machine to smoke-test them (thanks to Andy Simpkins). I forget
whether you're interested in fixing contrib or not, so I'm doing iortcw
as well as ioquake3 (openjk is experimental-only so is not relevant here).
Let me know if I should redirect the iortcw update to the release team.

Here is some text which might be useful for a DSA:

8<

A read buffer overflow was discovered in the idtech3 (Quake III Arena)
family of game engines. This allows remote attackers to cause a denial
of service (application crash) or possibly have unspecified other impact
via a crafted packet. (CVE-2017-11721)

In Debian, this issue affects the ioquake3, iortcw and openjk packages.

For the stable distribution (stretch), this issue has been fixed in
ioquake3 version 1.36+u20161101+dfsg1-2+deb9u1 and in iortcw version
1.50a+dfsg1-3+deb9u1.

In the unstable distribution (sid), this issue has been fixed in ioquake3
version 1.36+u20170803+dfsg1-1 and in iortcw version 1.51+dfsg1-3.

In the experimental distribution this issue has been fixed in openjk
version 0~20170718+dfsg1-2.

>8

Proposed debdiffs attached. OK to upload?

Regards,
S
diffstat for iortcw-1.50a+dfsg1 iortcw-1.50a+dfsg1

 changelog|8 
 patches/debian/Remove-support-for-downloading-executable-updates.patch   |2 
 patches/security/All-Fix-improve-buffer-overflow-in-MSG_ReadBits-MSG_Write.patch |  626 ++
 patches/series   |1 
 4 files changed, 636 insertions(+), 1 deletion(-)

diff -Nru iortcw-1.50a+dfsg1/debian/changelog iortcw-1.50a+dfsg1/debian/changelog
--- iortcw-1.50a+dfsg1/debian/changelog	2017-03-14 05:37:19.0 -0400
+++ iortcw-1.50a+dfsg1/debian/changelog	2017-08-08 14:57:52.0 -0400
@@ -1,3 +1,11 @@
+iortcw (1.50a+dfsg1-3+deb9u1) stretch-security; urgency=medium
+
+  * d/p/security/All-Fix-improve-buffer-overflow-in-MSG_ReadBits-MSG_Write.patch:
+Add patch (from ioquake3 via upstream) to fix a read buffer overflow
+in MSG_ReadBits (CVE-2017-11721)
+
+ -- Simon McVittie   Tue, 08 Aug 2017 14:57:52 -0400
+
 iortcw (1.50a+dfsg1-3) unstable; urgency=high
 
   * d/gbp.conf: switch branch to debian/stretch for updates during freeze
diff -Nru iortcw-1.50a+dfsg1/debian/patches/debian/Remove-support-for-downloading-executable-updates.patch iortcw-1.50a+dfsg1/debian/patches/debian/Remove-support-for-downloading-executable-updates.patch
--- iortcw-1.50a+dfsg1/debian/patches/debian/Remove-support-for-downloading-executable-updates.patch	2017-03-14 05:37:19.0 -0400
+++ iortcw-1.50a+dfsg1/debian/patches/debian/Remove-support-for-downloading-executable-updates.patch	2017-08-08 14:57:52.0 -0400
@@ -219,7 +219,7 @@
  // DHM - Nerve
  
 diff --git a/MP/code/qcommon/qcommon.h b/MP/code/qcommon/qcommon.h
-index 1f23d0f..b1fff59 100644
+index 02ef8e8..96a9081 100644
 --- a/MP/code/qcommon/qcommon.h
 +++ b/MP/code/qcommon/qcommon.h
 @@ -1258,12 +1258,6 @@ void Sys_StartProcess( char *cmdline, qboolean doexit );// NERVE - S
diff -Nru iortcw-1.50a+dfsg1/debian/patches/security/All-Fix-improve-buffer-overflow-in-MSG_ReadBits-MSG_Write.patch iortcw-1.50a+dfsg1/debian/patches/security/All-Fix-improve-buffer-overflow-in-MSG_ReadBits-MSG_Write.patch
--- iortcw-1.50a+dfsg1/debian/patches/security/All-Fix-improve-buffer-overflow-in-MSG_ReadBits-MSG_Write.patch	1969-12-31 19:00:00.0 -0500
+++ iortcw-1.50a+dfsg1/debian/patches/security/All-Fix-improve-buffer-overflow-in-MSG_ReadBits-MSG_Write.patch	2017-08-08 14:57:52.0 -0400
@@ -0,0 +1,626 @@
+From: MAN-AT-ARMS 
+Date: Thu, 3 Aug 2017 00:06:37 -0400
+Subject: All: Fix/improve buffer overflow in MSG_ReadBits/MSG_WriteBits
+
+Origin: upstream, commit:260c39a29af517a08b3ee1a0e78ad654bdd70934
+Bug-CVE: CVE-2017-11721
+Bug-Debian: https://bugs.debian.org/870811
+---
+ MP/code/qcommon/huffman.c | 49 ---
+ MP/code/qcommon/msg.c | 45 +---
+ MP/code/qcommon/qcommon.h |  6 ++---
+ SP/code/qcommon/huffman.c | 49 ---
+ SP/code/qcommon/msg.c | 65 ++-
+ SP/code/qcommon/qcommon.h |  6 ++---
+ 6 files changed, 145 insertions(+), 75 deletions(-)
+
+diff --git a/MP/code/qcommon/huffman.c b/MP/code/qcommon/huffman.c
+index 00b007e..88b972c 100644
+--- a/MP/code/qcommon/huffman.c
 b/MP/code/qcommon/huffman.c
+@@ -36,7 +36,7 @@ If you have questions concerning this license o

Processed: Re: Bug#870725: CVE-2017-11721: read buffer overflow in MSG_ReadBits

2017-08-05 Thread Debian Bug Tracking System
Processing control commands:

> clone -1 -2
Bug #870725 {Done: Simon McVittie } [src:ioquake3] 
CVE-2017-11721: read buffer overflow in MSG_ReadBits
Bug 870725 cloned as bug 870811
> reassign -2 src:iortcw
Bug #870811 {Done: Simon McVittie } [src:ioquake3] 
CVE-2017-11721: read buffer overflow in MSG_ReadBits
Bug reassigned from package 'src:ioquake3' to 'src:iortcw'.
No longer marked as found in versions ioquake3/1.36+u20170720+dfsg1-1.
No longer marked as fixed in versions ioquake3/1.36+u20170803+dfsg1-1.
> forwarded -2 
> https://github.com/iortcw/iortcw/commit/260c39a29af517a08b3ee1a0e78ad654bdd70934
Bug #870811 {Done: Simon McVittie } [src:iortcw] 
CVE-2017-11721: read buffer overflow in MSG_ReadBits
Changed Bug forwarded-to-address to 
'https://github.com/iortcw/iortcw/commit/260c39a29af517a08b3ee1a0e78ad654bdd70934'
 from 
'https://github.com/ioquake/ioq3/commit/d2b1d124d4055c2fcbe5126863487c52fd58cca1'.
> found -2 1.51+dfsg1-2
Bug #870811 {Done: Simon McVittie } [src:iortcw] 
CVE-2017-11721: read buffer overflow in MSG_ReadBits
Marked as found in versions iortcw/1.51+dfsg1-2 and reopened.
> fixed -2 1.51+dfsg1-3
Bug #870811 [src:iortcw] CVE-2017-11721: read buffer overflow in MSG_ReadBits
The source 'iortcw' and version '1.51+dfsg1-3' do not appear to match any 
binary packages
Marked as fixed in versions iortcw/1.51+dfsg1-3.

-- 
870725: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=870725
870811: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=870811
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems



Bug#870725: CVE-2017-11721: read buffer overflow in MSG_ReadBits

2017-08-05 Thread Simon McVittie
Control: clone -1 -2
Control: reassign -2 src:iortcw
Control: forwarded -2 
https://github.com/iortcw/iortcw/commit/260c39a29af517a08b3ee1a0e78ad654bdd70934
Control: found -2 1.51+dfsg1-2
Control: fixed -2 1.51+dfsg1-3

On Sat, 05 Aug 2017 at 11:47:23 +0100, Simon McVittie wrote:
> On Fri, 04 Aug 2017 at 16:30:46 +0200, Moritz Muehlenhoff wrote:
> > Please see https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-11721

iortcw in contrib also has this. I've uploaded a fix.

Again, I don't have time to handle this for stable right now, so
security or games team members are very welcome to do so. I'll prepare
a stable update during Debconf if nobody gets there first, assuming I
can find a stable user willing to test a game from contrib.

S



Bug#870725: CVE-2017-11721: read buffer overflow in MSG_ReadBits

2017-08-05 Thread Simon McVittie
Control: retitle -1 CVE-2017-11721: read buffer overflow in MSG_ReadBits
Control: tags -1 + upstream fixed-upstream patch
Control: forwarded -1 
https://github.com/ioquake/ioq3/commit/d2b1d124d4055c2fcbe5126863487c52fd58cca1

On Fri, 04 Aug 2017 at 16:30:46 +0200, Moritz Muehlenhoff wrote:
> Please see https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-11721

I have fixed this in unstable with a newer upstream snapshot. I suspect
that the bug is also present in all older suites, but I have not had
time to research that. Any suite where the upstream commit cherry-picks
successfully is probably vulnerable.

I am travelling (to Debconf) and finishing writing a talk, so I will
be unable to address this in older suites for now. If someone from the
security or games team wants to prepare and upload a backport of the
commit referenced by MITRE, please go ahead. From the commit message
and a quick read through the code, my understanding is that only the
MSG_ReadBits side is security-sensitive, with the MSG_WriteBits side
being merely for correctness (the buffer overflow check is too
pessimistic and will sometimes report an overflow when there are in
fact a few bytes left); but I could be wrong, and taking the entire
commit is probably the safer option.

The debian/stretch and debian/jessie branches in
https://anonscm.debian.org/git/pkg-games/ioquake3.git should be up to
date, and that git repository also contains the upstream commit
d2b1d124d4055c2fcbe5126863487c52fd58cca1.

Otherwise, I'll come back to this after I've given my my talk at Debconf,
assuming I can recruit someone running stable to smoke-test the new
version.

Thanks,
S



Processed: Re: Bug#870725: CVE-2017-11721: read buffer overflow in MSG_ReadBits

2017-08-05 Thread Debian Bug Tracking System
Processing control commands:

> retitle -1 CVE-2017-11721: read buffer overflow in MSG_ReadBits
Bug #870725 {Done: Simon McVittie } [src:ioquake3] 
CVE-2017-11721
Changed Bug title to 'CVE-2017-11721: read buffer overflow in MSG_ReadBits' 
from 'CVE-2017-11721'.
> tags -1 + upstream fixed-upstream patch
Bug #870725 {Done: Simon McVittie } [src:ioquake3] 
CVE-2017-11721: read buffer overflow in MSG_ReadBits
Added tag(s) fixed-upstream, patch, and upstream.
> forwarded -1 
> https://github.com/ioquake/ioq3/commit/d2b1d124d4055c2fcbe5126863487c52fd58cca1
Bug #870725 {Done: Simon McVittie } [src:ioquake3] 
CVE-2017-11721: read buffer overflow in MSG_ReadBits
Set Bug forwarded-to-address to 
'https://github.com/ioquake/ioq3/commit/d2b1d124d4055c2fcbe5126863487c52fd58cca1'.

-- 
870725: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=870725
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems