Re: widelands security fix, 617960
On Mon, Mar 14, 2011 at 07:57:56PM +0100, Moritz Mühlenhoff wrote:
> On Mon, Mar 14, 2011 at 11:40:59AM +0100, Enrico Tassi wrote:
> > Regarding
> >
> > http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=617960
> >
> > I'm a bit short of time, I didn't verify the bug either, but I prepared
> > an updated package, debdiff follows. You can also check it out with
> > debcheckout, I've just committed it.
>
> I don't think this warrants a DSA, please fix it through a stable
> point update instead. CCing the stable security point update coordinator.
OK, I'm CC-ing debian-release, since according to the developer
references it's the right place to discuss this upload.
I attach again the debdiff. Should I upload that package to
stable-proposed-updates then?
Cheers
--
Enrico Tassi
diff -Nru widelands-15/debian/changelog widelands-15/debian/changelog
--- widelands-15/debian/changelog 2010-05-10 15:58:16.000000000 +0200
+++ widelands-15/debian/changelog 2011-03-14 11:02:58.000000000 +0100
@@ -1,3 +1,10 @@
+widelands (1:15-2+squeeze1) stable-security; urgency=high
+
+ * Closes a potential security issue in internet games.
+ Added: patches/secfix-617960 (Closes: #617960)
+
+ -- Enrico Tassi <gareuselesinge@debian.org> Mon, 14 Mar 2011 11:01:49 +0100
+
widelands (1:15-2) unstable; urgency=low
* add patch s390 to fix FTBFS on s390. The patch defines
diff -Nru widelands-15/debian/patches/secfix-617960 widelands-15/debian/patches/secfix-617960
--- widelands-15/debian/patches/secfix-617960 1970-01-01 01:00:00.000000000 +0100
+++ widelands-15/debian/patches/secfix-617960 2011-03-14 10:52:58.000000000 +0100
@@ -0,0 +1,177 @@
+=== modified file 'ChangeLog'
+--- widelands-15/ChangeLog 2010-03-26 08:57:09 +0000
++++ widelands-15/ChangeLog 2011-03-05 15:40:23 +0000
+@@ -1,3 +1,6 @@
++### Build 15.1
++- Fixed a potential security issue in internet games
++
+ ### Build 15 rc2
+ - Removed registering functionality for metaserver. This is to be compatible
+ with future changes to the metaserver.
+
+=== modified file 'WL_RELEASE'
+--- widelands-15/WL_RELEASE 2010-04-10 16:16:12 +0000
++++ widelands-15/WL_RELEASE 2011-03-05 15:40:23 +0000
+@@ -1,1 +1,1 @@
+-build-15
++build-15.1
+
+=== modified file 'src/editor/ui_menus/editor_main_menu_new_map.cc'
+--- widelands-15/src/editor/ui_menus/editor_main_menu_new_map.cc 2009-11-22 23:03:13 +0000
++++ widelands-15/src/editor/ui_menus/editor_main_menu_new_map.cc 2011-03-05 15:40:23 +0000
+@@ -1,5 +1,5 @@
+ /*
+- * Copyright (C) 2002-2004, 2006-2009 by the Widelands Development Team
++ * Copyright (C) 2002-2004, 2006-2009, 2011 by the Widelands Development Team
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+@@ -113,7 +113,7 @@
+ posx, posy, width, height,
+ g_gr->get_picture(PicMod_UI, "pics/but1.png"),
+ &Main_Menu_New_Map::button_clicked, *this, 4,
+- Widelands::World::World(m_worlds[m_currentworld].c_str()).get_name());
++ Widelands::World(m_worlds[m_currentworld].c_str()).get_name());
+
+ posy += height + spacing + spacing + spacing;
+
+@@ -142,7 +142,7 @@
+ if (m_currentworld == m_worlds.size())
+ m_currentworld = 0;
+ m_world->set_title
+- (Widelands::World::World(m_worlds[m_currentworld].c_str()).get_name
++ (Widelands::World(m_worlds[m_currentworld].c_str()).get_name
+ ());
+ break;
+ }
+
+=== modified file 'src/editor/ui_menus/editor_main_menu_random_map.cc'
+--- widelands-15/src/editor/ui_menus/editor_main_menu_random_map.cc 2009-11-24 09:43:42 +0000
++++ widelands-15/src/editor/ui_menus/editor_main_menu_random_map.cc 2011-03-05 15:40:23 +0000
+@@ -1,5 +1,5 @@
+ /*
+- * Copyright (C) 2002-2004, 2006-2009 by the Widelands Development Team
++ * Copyright (C) 2002-2004, 2006-2009, 2011 by the Widelands Development Team
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+@@ -272,7 +272,7 @@
+ posx, posy, width, height,
+ g_gr->get_picture(PicMod_UI, "pics/but1.png"),
+ &Main_Menu_New_Random_Map::button_clicked, *this, 8,
+- Widelands::World::World(m_worlds[m_currentworld].c_str()).get_name());
++ Widelands::World(m_worlds[m_currentworld].c_str()).get_name());
+
+ posy += height + spacing + spacing + spacing;
+
+@@ -343,7 +343,7 @@
+ if (m_currentworld == m_worlds.size())
+ m_currentworld = 0;
+ m_world->set_title
+- (Widelands::World::World(m_worlds[m_currentworld].c_str()).get_name());
++ (Widelands::World(m_worlds[m_currentworld].c_str()).get_name());
+ break;
+ case 9:
+ break;
+@@ -476,7 +476,7 @@
+ (strcmp(mapInfo.worldName.c_str(), m_worlds[m_currentworld].c_str()))
+ ++m_currentworld;
+ m_world->set_title
+- (Widelands::World::World(m_worlds[m_currentworld].c_str()).get_name());
++ (Widelands::World(m_worlds[m_currentworld].c_str()).get_name());
+
+ button_clicked(-1); // Update other values in UI as well
+
+
+=== modified file 'src/io/filesystem/filesystem.cc'
+--- widelands-15/src/io/filesystem/filesystem.cc 2010-04-01 16:14:18 +0000
++++ widelands-15/src/io/filesystem/filesystem.cc 2011-03-05 15:40:23 +0000
+@@ -112,6 +112,10 @@
+ temp = fixedpath.at(i);
+ if (temp == "\\")
+ fixedpath.at(i) = m_filesep;
++ // As a security measure, eat all dots and tildes away when file is
++ // tranferred over network.
++ if (temp == "." || temp == "~")
++ fixedpath.at(i) = '-';
+ }
+ return fixedpath;
+ #endif
+
+=== modified file 'src/network/netclient.cc'
+--- widelands-15/src/network/netclient.cc 2010-04-01 12:08:39 +0000
++++ widelands-15/src/network/netclient.cc 2011-03-05 15:40:23 +0000
+@@ -35,6 +35,8 @@
+ #include "warning.h"
+ #include "wexception.h"
+ #include "wlapplication.h"
++#include "game_io/game_loader.h"
++#include "map_io/widelands_map_loader.h"
+
+ #include "ui_basic/messagebox.h"
+ #include "ui_basic/progresswindow.h"
+@@ -572,7 +574,7 @@
+ }
+ }
+ // Don't overwrite the file, better rename the original one
+- g_fs->Rename(path, "backup-" + path);
++ g_fs->Rename(path, backupFileName(path));
+ }
+
+ // Yes we need the file!
+@@ -659,6 +661,40 @@
+ s.Unsigned8(NETCMD_CHAT);
+ s.String(_("/me 's file failed md5 checksumming."));
+ s.send(d->sock);
++ g_fs->Unlink(file->filename);
++ }
++ // Check file for validity
++ bool invalid = false;
++ if (d->settings.savegame) {
++ // Saved game check - does Widelands recognize the file as saved game?
++ Widelands::Game game;
++ try {
++ Widelands::Game_Loader gl(file->filename, game);
++ } catch (...) {
++ invalid = true;
++ }
++ } else {
++ // Map check - does Widelands recognize the file as map?
++ Widelands::Map map;
++ Widelands::Map_Loader * const ml = map.get_correct_loader(file->filename.c_str());
++ if (!ml)
++ invalid = true;
++ }
++ if (invalid) {
++ g_fs->Unlink(file->filename);
++ // Restore original file, if there was one before
++ if (g_fs->FileExists(backupFileName(file->filename)))
++ g_fs->Rename(backupFileName(file->filename), file->filename);
++
++ /* TODO Uncomment after Build16 string freeze
++ s.reset();
++ s.Unsigned8(NETCMD_CHAT);
++ s.String
++ (_
++ ("/me checked the recieved file. Although md5 check summing succeded, "
++ "I can not handle the file."));
++ s.send(d->sock);
++ */
+ }
+ }
+ break;
+
+=== modified file 'src/network/netclient.h'
+--- widelands-15/src/network/netclient.h 2010-01-07 10:51:25 +0000
++++ widelands-15/src/network/netclient.h 2011-03-05 15:40:23 +0000
+@@ -88,6 +88,9 @@
+ std::vector<ChatMessage> const & getMessages() const;
+
+ private:
++ /// for unique backupname
++ std::string backupFileName(std::string & path) {return path + "~backup";}
++
+ NetTransferFile * file;
+
+ void syncreport();
+
diff -Nru widelands-15/debian/patches/series widelands-15/debian/patches/series
--- widelands-15/debian/patches/series 2010-05-10 15:55:12.000000000 +0200
+++ widelands-15/debian/patches/series 2011-03-14 10:50:35.000000000 +0100
@@ -1 +1,2 @@
s390
+secfix-617960
File lists identical (after any substitutions)
Control files of package widelands: lines which differ (wdiff format)
---------------------------------------------------------------------
Depends: widelands-data (= [-1:15-2),-] {+1:15-2+squeeze1),+} libc6 (>= 2.2.5), libgcc1 (>= 1:4.1.1), libggz2 (>= 0.0.14.1), libggzcore9 (>= 0.0.14.1), libggzmod4 (>= 0.0.14.1), [-libjpeg62,-] {+libjpeg62 (>= 6b1),+} liblua5.1-0, libpng12-0 (>= 1.2.13-4), [-libsdl-gfx1.2-4,-] {+libsdl-gfx1.2-4 (>= 2.0.19),+} libsdl-image1.2 (>= 1.2.10), libsdl-mixer1.2 (>= 1.2.6), libsdl-net1.2, libsdl-ttf2.0-0, libsdl1.2debian (>= 1.2.10-1), libstdc++6 (>= 4.4.0), libtiff4, zlib1g (>= 1:1.1.4), ttf-freefont
Version: [-1:15-2-] {+1:15-2+squeeze1+}
Control files of package widelands-data: lines which differ (wdiff format)
--------------------------------------------------------------------------
Version: [-1:15-2-] {+1:15-2+squeeze1+}
Control files of package widelands-dbg: lines which differ (wdiff format)
-------------------------------------------------------------------------
Depends: widelands (= [-1:15-2)-] {+1:15-2+squeeze1)+}
Installed-Size: [-47480-] {+47492+}
Version: [-1:15-2-] {+1:15-2+squeeze1+}
Reply to: