Wesnoth security fixes

Comparing the fixes that Debian performed[1], I think this patch may additionally require fixes for CVE-2009-0366. Also, please follow the changelog format in the Security Update Procedures[2], since that will make it easier for us to examine the patches.

I do have a worry that just ripping out Python is the wrong approach to take with this bug, as that drops features as well. However, in the light of upstream's response to the bug (they did the same), I think it makes sense. Will there be AIs that no longer work if this code is removed from wesnoth?

[1] http://packages.debian.org/changelogs/pool/main/w/wesnoth/current/changelog
[2] https://wiki.ubuntu.com/SecurityUpdateProcedures

Gutsy: http://launchpadlibrarian.net/23269826/wesnoth_1.2.6-1ubuntu2.5.interdiff.gz

* Kees Cook <email address hidden> [2009-03-10 19:09:41 CET]:
> Comparing the fixes that Debian performed[1], I think this patch may
> additionally require fixes for CVE-2009-0366.

 Not in this version because it doesn't support gzip compression, so the
changes aren't that big. Additionally, the code has changed a fair bit
through the gzip compression addition and I wasn't able to locate the
specific code sections with some quick search. But given that the
execution of arbitrary code on client machines is a fair bit more severe
than this issue I think it's more than acceptable to _not_ delay the
python fix any further.

> Also, please follow the changelog format in the Security Update
> Procedures[2], since that will make it easier for us to examine the
> patches.

 I'm sorry, I'm no MOTU, I can't sign the Ubuntu CoC because with that I
would claim to expect Mark to be perfect, so I haven't tried to dig
further into Ubuntu practises. I just wanted to offer in a best-effort
manner what could help you to get the problem fixed. If you are fine
with letting the users vulnerable to arbitrary code execution that's
fine with me.

 And I'm not completely sure why the style of the changelog should
actually make it easier to examine the patch - but then, that might be
just me.

> I do have a worry that just ripping out Python is the wrong approach to
> take with this bug, as that drops features as well.

 It drops a feature that is dropped in the upcoming 1.6 release and
never really was used anywhere at all but only a single scenario in a
single campaign (which was fixed with the update too, take a look at the
changes that went into jaunty). All user created stuff that is available
through the wesnoth addon server was checked, and given that the
community is pretty tight this is a quite complete check.

 Furthermore, if you know a way to properly sandbox python, feel free to
bring it up. The wesnoth team has quite some python evangelists on board
but they were unable to find a way to sanely limit python. Additionally,
it is discussed to replace this feature with a scripting language that
is actually intended to be run in such an environment - so it might come
back eventually (even if it wasn't really used ...).

> However, in the light of upstream's response to the bug (they did the
> same), I think it makes sense. Will there be AIs that no longer work
> if this code is removed from wesnoth?
>
> [1] http://packages.debian.org/changelogs/pool/main/w/wesnoth/current/changelog

 You might rather want to take a look of this changelog (the one that
went into unstable, not experimental, and through that into jaunty):
http://packages.debian.org/changelogs/pool/main/w/wesnoth/wesnoth_1.4.7-4/changelog

 Quoting from that changelog, on which most parts of the both patches
are based:

     - Pull wesnoth-did-ai-fix patch from upstream svn r33013 to make it still
       work after above changes.

 I did like I was adviced, and I'm sorry that it doesn't please you
enough, but I have done six uploads/patches for wesnoth in Debian,
spoken to Secunia to answer their questions about the issue, coordinated
with the wesnoth team directly, prepared these two patches (whic...

> I did like I was adviced, and I'm sorry that it doesn't please you enough

This isn't so much about 'pleasing us enough' as about ensuring the patch is correct and well tested. This package is in universe and is community supported and as such, needs caring people like yourself to tend to it. I'm reviewing the patches today and will report back if I have any questions.

Based on your feedback I am going to push the updates for Gutsy and Hardy today. If you are able, perhaps you could prepare a debdiff to fix both CVE-2009-0366 and CVE-2009-0367 on Intrepid. Thanks for your hard work on this!

* Jamie Strandboge <email address hidden> [2009-03-19 15:28:42 CET]:
> > I did like I was adviced, and I'm sorry that it doesn't please you
> enough
>
> This isn't so much about 'pleasing us enough' as about ensuring the
> patch is correct and well tested.

 I can't completely follow. Is six different patched versions that I did
for Debian (1.2, 1.4.4 (twice in different environment), 1.4.7 (twice in
different environment), 1.5.11) not well enough tested for you? Is the
acceptance by the Debian security team in a DSA not well enough tested
for you?

> This package is in universe and is community supported and as such,
> needs caring people like yourself to tend to it.

 I really thought I did so - but it's your call to accept it, not mine.

> I'm reviewing the patches today and will report back if I have any
> questions.

 Sure, you're welcome. Btw., for completeness:

#v+
+ - Pull limit-mapsize patch from upstream svn r32987 to avoid hanging of
+ wesnoth/exhausting system memory (Upstream Bug #13031)
#v-

 This part in the changelog received a CVE ID in the meantime:
CVE-2009-0878 - you might want to incorporate that into the changelog.

 I didn't prepare a patch for intrepid because I was told that there is
an update in intrepid-proposed already and I haven't received any
informations about on which version I should base the patch on - the one
actually _in_ intrepid, or the version in intrepid-proposed. About the
backports I guess it's just similar to Debian backports: Taking the new
(security) upload and building it for there, so most propably no patch
needed there, right?

 I also know that wesnoth is in universe and don't receive regular
security support - on the other hand, this issue isn't a random DoS but
a arbitrary code execution issue that shouldn't get ignored like it was
in the last weeks. I'm quite a bit disappointed here both with the
delays and with the kind of responses, to say the least. :/

 Thanks anyway.
Rhonda

Rhonda, no one is criticizing your work. To the contrary, you have done great work with this. The aforementioned testing is needed to be done on Ubuntu because Ubuntu != Debian. Granted, it will most likely work ok but there have been quite a few times when a tested Debian patch didn't work properly on Ubuntu, due to different versions.

The Ubuntu Security team does not have the resources to patch and test software in universe and multiverse and relies on MOTU Swat and people from the community to maintain this software. I apologize for the delays and am working to get these fixes into Ubuntu.

As for Intrepid, please patch the current version as released in Ubuntu, and add a note to the SRU bug for the -proposed package saying it has to be updated to incorporate these security fixes. Please use a version higher than that in -proposed, following the guidelines in https://wiki.ubuntu.com/SecurityUpdateProcedures.

Also, if you decide to provide a debdiff for Intrepid, please mark the Intrepid task in this bug as 'In Progress'. Thanks.

wesnoth (1:1.4-1ubuntu0.1) hardy-security; urgency=low

  * Upload to fix several severe problems:
    - Compile with --disable-python because the python AI support allowed to
      break out of sandbox and allowed execution of abitrary code
      (CVE-2009-0367, Upstream Bug #13048). Don't install data/ais into
      wesnoth-data package anymore, and remove python-dev from
      Build-Dependencies.
    - Pull wesnoth-did-ai-fix patch from upstream svn r33013 to make it still
      work after above changes.
    - Pull limit-mapsize patch from upstream svn r32987 to avoid hanging of
      wesnoth/exhausting system memory (Upstream Bug #13031)

 -- Gerfried Fuchs < <email address hidden>> Sun, 01 Mar 2009 21:05:56 +0100

wesnoth (1.2.6-1ubuntu2.5) gutsy-security; urgency=low

  * Upload to fix a severe problem:
    - Compile with --disable-python because the python AI support allowed to
      break out of sandbox and allowed execution of abitrary code
      (CVE-2009-0367, Upstream Bug #13048). Remove python-dev from
      Build-Dependencies.

I backported the fixes from Jaunty and created the following debdiff and am building it. It would be great if someone could take a look at it and make sure it is ok.

* Jamie Strandboge <email address hidden> [2009-03-20 15:05:44 CET]:
> I backported the fixes from Jaunty and created the following debdiff and
> am building it. It would be great if someone could take a look at it and
> make sure it is ok.
>
> ** Attachment added: "wesnoth_1.4.5-1ubuntu0.1.debdiff"
> http://launchpadlibrarian.net/24149791/wesnoth_1.4.5-1ubuntu0.1.debdiff

 Thanks, this looks almost identical to the patch I was working on. Your
last mail motivated me to give it a try and I just didn't check for the
specific format wishes for the changelog from the wiki page yet - was
busy with getting the 1.6a hotfix release uploaded to Debian and planed
to do it afterwards.

 I can confirm that the patch is technically the same that I would have
proposed. Really big thanks to you Jamie for picking it up.

 All the best to you, and again: thanks!
Rhonda

Heh, I forgot to check -proposed. Here is an updated debdiff.

Thanks Rhonda! I'll get this out today then.

This bug was fixed in the package wesnoth - 1:1.4.5-1ubuntu0.2

---------------
wesnoth (1:1.4.5-1ubuntu0.2) intrepid-security; urgency=low

  * SECURITY UPDATE: arbitrary code execution via python AI (LP: #336396)
    - debian/control: remove python-dev from Build-Dependencies
    - debian/rules: Compile with --disable-python
    - debian/wesnoth-data.install: Don't install data/ais into
    - debian/patches/04wesnoth-did-ai-fix: upstream svn r33013 for above
      changes
    - Patch based on work by Gerfried Fuchs
    - CVE-2009-0367
  * SECURITY UPDATE: denial of service large compressed WML document
    - debian/patches/03fix-server-dos: check size of WML document in
      simple_wml.cpp
    - Patch based on work by Gerfried Fuchs
    - CVE-2009-0366
  * SECURITY UPDATE: denial of service via crafted map
    - debian/patches/05limit-mapsize: verify map size in
      terrain_translation.cpp and terrain_translation.hpp
    - Patch based on work by Gerfried Fuchs
    - CVE-2009-0878

 -- Jamie Strandboge <email address hidden> Fri, 20 Mar 2009 08:35:09 -0500

Here is the Gutsy debdiff from the duplicate.