Bug#780327: unblock: armagetronad/0.2.8.3.2-4
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: unblock
Dear release team,
please unblock package armagetronad.
A new upstream release of ArmagetronAD was released a few days ago [1]
which contained fixes for security vulnerabilities of client and
server and possible application crashes.
I asked the main developer of ArmagetronAD to provide a minimal patch
that only addresses the most important issues and I would like to ship
those updates in Jessie.
This update fixes:
- a remotely exploitable bug in network error handling. In client mode,
any received packet that causes an exception during processing could
terminate the connection to the server. Game servers were also vulnerable
during the brief period while they are communicating with the master servers,
and the effect then is that the server will not advertise itself.
- very short UDP packets could cause a read beyond the input buffer
- potential crash with friend list filtering
- rare crash with sound lock
I deliberately kept upstream's changelog and NEWS entry because it clarifies
what the patch actually does and I did not have to duplicate the
information in the patch header.
The debdiff against the version of ArmagetronAD in Jessie is attached.
unblock armagetronad/0.2.8.3.2-4
Regards,
Markus
[1] http://armagetronad.org/
diff -Nru armagetronad-0.2.8.3.2/debian/changelog armagetronad-0.2.8.3.2/debian/changelog
--- armagetronad-0.2.8.3.2/debian/changelog 2014-12-06 11:59:00.000000000 +0100
+++ armagetronad-0.2.8.3.2/debian/changelog 2015-03-10 08:03:38.000000000 +0100
@@ -1,3 +1,11 @@
+armagetronad (0.2.8.3.2-4) unstable; urgency=medium
+
+ * Add security.patch and fix possible remotely exploitable
+ security vulnerabilities that could terminate network connections
+ of client and server. (Closes: #780178)
+
+ -- Markus Koschany <apo@gambaru.de> Tue, 10 Mar 2015 08:00:31 +0100
+
armagetronad (0.2.8.3.2-3) unstable; urgency=medium
* Fix all remaining bashisms in armagetronad-dedicated wrapper script by
diff -Nru armagetronad-0.2.8.3.2/debian/patches/security.patch armagetronad-0.2.8.3.2/debian/patches/security.patch
--- armagetronad-0.2.8.3.2/debian/patches/security.patch 1970-01-01 01:00:00.000000000 +0100
+++ armagetronad-0.2.8.3.2/debian/patches/security.patch 2015-03-10 08:03:38.000000000 +0100
@@ -0,0 +1,152 @@
+From: Markus Koschany <apo@gambaru.de>
+Date: Tue, 10 Mar 2015 07:29:18 +0100
+Subject: security
+
+---
+ ChangeLog | 18 ++++++++++++++++--
+ NEWS | 9 +++++++++
+ src/engine/eGameObject.cpp | 3 +++
+ src/network/nNetwork.cpp | 21 +++++++++++++++++----
+ src/tron/gServerBrowser.cpp | 2 +-
+ 5 files changed, 46 insertions(+), 7 deletions(-)
+
+diff --git a/ChangeLog b/ChangeLog
+index d13b1cf..f2d5d33 100644
+--- a/ChangeLog
++++ b/ChangeLog
+@@ -1,7 +1,21 @@
+ ------------------------------------------------------------------------
+-r9916 | z-man | 2011-09-11 12:41:12 +0200 (Sun, 11 Sep 2011) | 2 lines
++r10712 | z-man | 2015-01-29 23:53:15 +0100 (Thu, 29 Jan 2015) | 2 lines
+
+-Injecting source again.
++Tagging 0.2.8.3.3_rc1
++
++------------------------------------------------------------------------
++r10706 | z-man | 2015-01-29 23:39:48 +0100 (Thu, 29 Jan 2015) | 1 line
++
++Merging fixes for various potential bugs from 0.2.8.
++------------------------------------------------------------------------
++r10505 | z-man | 2013-01-29 23:12:03 +0100 (Tue, 29 Jan 2013) | 2 lines
++
++Fixing possible crash due to friends list buffer overrun; no exploit potential.
++
++------------------------------------------------------------------------
++r10393 | z-man | 2012-03-31 17:59:04 +0200 (Sat, 31 Mar 2012) | 2 lines
++
++Backporting rare crashfix: Adding sound lock when alpha objects get resorted.
+
+ ------------------------------------------------------------------------
+ r9914 | z-man | 2011-09-11 12:40:11 +0200 (Sun, 11 Sep 2011) | 2 lines
+diff --git a/NEWS b/NEWS
+index 8347682..96f6abb 100644
+--- a/NEWS
++++ b/NEWS
+@@ -1,3 +1,12 @@
++Changes since 0.2.8.3.2:
++- security fix: do not read ahead of the beginning of network buffer.
++- security fix: don't attribute network errors from processing random
++ packets to the connection to the server
++- security fix: while at it, don't process random packets unless they
++ may be important
++- fix for potential crash with friend list filtering
++- fix for rare crash with sound lock
++
+ Changes since 0.2.8.3.1:
+ - security fix: old style action commands from clients no loger cause hangs and crashes
+ - security fix: oversized packets are ignored properly
+diff --git a/src/engine/eGameObject.cpp b/src/engine/eGameObject.cpp
+index 7e11b2f..64d3138 100644
+--- a/src/engine/eGameObject.cpp
++++ b/src/engine/eGameObject.cpp
+@@ -880,6 +880,9 @@ void eGameObject::RenderAll(eGrid *grid, const eCamera *cam){
+ // but the small flickering error is to be tolerated, especially
+ // since alpha blended game objects tend to gently fade in.
+ int firstAlphaID = firstAlpha->id;
++
++ eSoundLocker locker;
++
+ grid->gameObjects.Remove(firstAlpha,firstAlpha->id);
+ grid->gameObjects.Add(firstAlpha,firstAlpha->id);
+ grid->gameObjects.Remove(object,object->id);
+diff --git a/src/network/nNetwork.cpp b/src/network/nNetwork.cpp
+index 1628f30..5cc9c86 100644
+--- a/src/network/nNetwork.cpp
++++ b/src/network/nNetwork.cpp
+@@ -1413,6 +1413,10 @@ nServerInfoBase * sn_PeekRedirectTo()
+ }
+
+ void login_deny_handler(nMessage &m){
++ // only the server is allowed to send this
++ if(m.SenderID() != 0)
++ return;
++
+ if ( !m.End() )
+ {
+ // tOutput output;
+@@ -1908,6 +1912,11 @@ void logout_handler(nMessage &m){
+ unsigned short id = m.SenderID();
+ //m.Read(id);
+
++ // only the server or legal clients are allowed to send this
++ // (client check comes later)
++ if(sn_GetNetState() == nCLIENT && id != 0)
++ return;
++
+ if (sn_Connections[id].socket)
+ {
+ tOutput o;
+@@ -2266,7 +2275,7 @@ static void rec_peer(unsigned int peer){
+ nAddress addrFrom; // the sender of the current packet
+ len = sn_Connections[peer].socket->Read( reinterpret_cast<int8 *>(buff),maxrec*2, addrFrom);
+
+- if (len>0){
++ if (len>=2){
+ if ( len >= maxrec*2 )
+ {
+ #ifndef DEDICATED
+@@ -2360,6 +2369,10 @@ static void rec_peer(unsigned int peer){
+ }
+ else
+ {
++ // logged in clients should ignore packets from unknown sources
++ if(sn_GetNetState() != nSERVER && sn_myNetID != 0)
++ continue;
++
+ // assume it's a new connection
+ id = MAXCLIENTS+1;
+ peers[ MAXCLIENTS+1 ] = addrFrom;
+@@ -2491,7 +2504,7 @@ static void rec_peer(unsigned int peer){
+ catch(nKillHim)
+ {
+ con << "nKillHim signal caught: ";
+- sn_DisconnectUser(peer, "$network_kill_error");
++ sn_DisconnectUser(id, "$network_kill_error");
+ }
+ #endif
+ }
+@@ -3332,9 +3345,9 @@ void sn_DisconnectUser(int i, const tOutput& reason, nServerInfoBase * redirectT
+ }
+
+ // clients can only disconnect from the server
+- if ( i != 0 && sn_GetNetState() == nCLIENT )
++ if ( i != 0 && i <= MAXCLIENTS && sn_GetNetState() == nCLIENT )
+ {
+- tERR_ERROR( "Client tried to disconnect from another client: impossible and a bad idea." );
++ tERR_WARN( "Client tried to disconnect from another client: impossible and a bad idea." );
+ return;
+ }
+
+diff --git a/src/tron/gServerBrowser.cpp b/src/tron/gServerBrowser.cpp
+index 14e92a4..e26db42 100644
+--- a/src/tron/gServerBrowser.cpp
++++ b/src/tron/gServerBrowser.cpp
+@@ -392,7 +392,7 @@ void gServerMenu::Update()
+ int i;
+ tString userNames = run->UserNames();
+ tString* friends = getFriends();
+- for (i = MAX_FRIENDS; i>=0; i--)
++ for (i = MAX_FRIENDS-1; i>=0; i--)
+ {
+ if (run->Users() > 0 && friends[i].Len() > 1 && userNames.StrPos(friends[i]) >= 0)
+ {
diff -Nru armagetronad-0.2.8.3.2/debian/patches/series armagetronad-0.2.8.3.2/debian/patches/series
--- armagetronad-0.2.8.3.2/debian/patches/series 2014-12-06 11:59:00.000000000 +0100
+++ armagetronad-0.2.8.3.2/debian/patches/series 2015-03-10 08:03:38.000000000 +0100
@@ -1 +1,2 @@
desktop-file.patch
+security.patch
Reply to: