Insecure use of tarfile module PRIOR to validation of the downloaded tarfile

[David]

The way DistUpgrade/DistUpgradeFetcherCore.py uses tarfile is dangerous ...
The python documentation for tarfile[0] has a warning which states:
'Warning Never extract archives from untrusted sources without prior inspection. It is possible that files are created outside of path, e.g. members that have absolute filenames starting with "/" or filenames with two dots "..". '

However, the code flow does the following under run()
#1 download the release tar file ... via
   if not self.fetchDistUpgrader():

then it runs
#2 the vulnerable tarfile code via calling
if not self.extractDistUpgrader():

#3 after which it verifies the upgrade files ...
        if not self.verifyDistUprader():

In the extractDistUpgrader method the vulnerable use of tarfile as follows:
    def extractDistUpgrader(self):
          # extract the tarbal
          fname = os.path.join(self.tmpdir,os.path.basename(self.uri))
          print "extracting '%s'" % os.path.basename(fname)
          if not os.path.exists(fname):
              return False
          try:
              tar = tarfile.open(self.tmpdir+"/"+os.path.basename(self.uri),"r")
              for tarinfo in tar:
                  tar.extract(tarinfo)
              tar.close()

As the tar.extract method is called on the 'tarinfo' which is not 'checked' or guarded against ../'s (path traversal) containing file-names it would appear that the code is vulnerable to path traversal ...

[0] http://docs.python.org/library/tarfile.html#tarfile.TarFile.extract

[Marc Deslauriers (mdeslaur)]

Michael, could you please take a look and confirm? Thanks.

[Michael Vogt (mvo)]

Thanks for your bugreport and sorry for my slow reply. I was traveling during the previous days.

Your analysis is exactly right, this code is broken, I attach a fix.

[Michael Vogt (mvo)]

The previous patch is not sufficient. This new one fixes a additional MitM attack where a attacker can
give a modified meta-release file to bypass the authentication. This makes the previous one obsolete.

[Marc Deslauriers (mdeslaur)]

Thanks for reporting this, David.

Thanks for the patch, Michael.

This is CVE-2011-3152.

I will work on updates for this issue next week, after UDS.

[David (d--)]

I wasn't sure if this code was actually used or not. It looks like when a cd of a new release is inserted the code in DistUpgradeController will run --> up and into the prepare method if the user selects the option to get new updates from the internet then the self._tryUpdateSelf method is called which does this -->

        from DistUpgradeFetcherSelf import DistUpgradeFetcherSelf
where DistUpgradeFetcherSelf is a subclass of DistUpgradeFetcherCore -- and the _tryUpdateSelf will create and invoke the run() (vulnerable) method of DistUpgradeFetcherSelf(which is inherited from DistUpgradeFetcherCore).

If this is correct it means that cd triggered updates may permit a remote attacker to remotely compromise a vulnerable machine during the upgrade process if the option to upgrade from the network is selected.

[Michael Vogt (mvo)]

Hey David, this is indeed the case :( What we can do is to fix update-notifier that will watch for the cdrom insert events and runs the upgrade. We need to add a step here that automatically patches the broken code. I will work on this shortly (I'm currently at the Ubuntu Developer Conference).

For the version in precise we need to a) fix the code b) move cdromupgrade into update-manager-core so that we can ship security updates for it to networked systems independently from the CD.

The case that is not handled with the update-notifier fix outlined above is when someone runs the cdromupgrade script on the CD by hand. I don't really see a way currently to fix this except for patch python itself and fail if it encounters the DistUpgradeFetcherCore.py file with a certain sha1.

[David (d--)]

No problem :-)

> I don't really see a way currently to fix this except for patch python itself and fail if it encounters the
> DistUpgradeFetcherCore.py file with a certain sha1.
Right so you could do a comparison using os.path.realpath for where the file from the tar would end up and skip evil file-names. I don't think this is a real issue (you could remove the __init__ code ... to make it harder to call if no one is using it :-) ).

[David (d--)]

erh s/__init__/__name__/ (as in =="__main__")

[Michael Vogt (mvo)]

Sorry again for the slowness, I'm back from UDS now, still fighting jetlag, but mostly good.
The attached patch should fix most people who use the normal way of doing a cdromupgrade via the update-notifier
prompt.

I did not actually test that yet, but once I got a CD and the download is done I will.

[David (d--)]

Is there a timeline on a release fix for this issue. Also I noticed that /usr/bin/do-release-upgrade appears to run into the vulnerable code as well (during an update).

[David (d--)]

s/Is there a timeline on a release fix for this issue/When will a fix be released\?/

[Marc Deslauriers (mdeslaur)]

Michael, when you do think you'll be able to test your proposed fix?

[Michael Vogt (mvo)]

I tested the patch now and it turns out that there is a typo in it.

Once that got fixed it works for me for:
natty->oneiric: yes
maverick->natty: yes
lucid->maverick: yes (attached patch has a trivial conflict though)

what I did was to get the natty/maverick/lucid update-notitifer, patch it, build it, insert a cdrom and verify that the update-notifier would patch the upgrader before it executes it.

[Michael Vogt (mvo)]

And again appologize for my slowness in getting back with the testing.

[Launchpad Janitor (janitor)]

This bug was fixed in the package update-manager - 1:0.152.25.5

---------------
update-manager (1:0.152.25.5) oneiric-security; urgency=low

  * SECURITY UPDATE: arbitrary code execution via directory traversal
    (LP: #881548)
    - UpdateManager/Core/DistUpgradeFetcherCore.py: verify signature before
      unpacking the tarball.
    - CVE-2011-3152
  * SECURITY UPDATE: information leak via insecure temp file (LP: #881541)
    - DistUpgrade/DistUpgradeViewKDE.py: use mkstemp instead of mktemp.
    - CVE-2011-3154
 -- Marc Deslauriers <email address hidden> Wed, 23 Nov 2011 08:52:19 -0500

[Launchpad Janitor (janitor)]

This bug was fixed in the package update-notifier - 0.111ubuntu2.1

---------------
update-notifier (0.111ubuntu2.1) natty-security; urgency=low

  * SECURITY UPDATE: hotfix for arbitrary code execution via directory
    traversal in update-manager on iso media (LP: #881548)
    - data/cddistupgrader: patch update-manager that is pulled off an
      upgrade cd.
    - debian/update-manager-downloader-fix2.diff: hotfix to verify
      signature before unpacking the tarball in
      UpdateManager/Core/DistUpgradeFetcherCore.py.
    - debian/update-notifier-common.*: ship new hotfix in package.
    - CVE-2011-3152
 -- Marc Deslauriers <email address hidden> Thu, 24 Nov 2011 12:57:39 -0500

[Launchpad Janitor (janitor)]

This bug was fixed in the package update-manager - 1:0.150.5.1

---------------
update-manager (1:0.150.5.1) natty-security; urgency=low

  * SECURITY UPDATE: arbitrary code execution via directory traversal
    (LP: #881548)
    - UpdateManager/Core/DistUpgradeFetcherCore.py: verify signature before
      unpacking the tarball.
    - CVE-2011-3152
  * SECURITY UPDATE: information leak via insecure temp file (LP: #881541)
    - DistUpgrade/DistUpgradeViewKDE.py: use mkstemp instead of mktemp.
    - CVE-2011-3154
 -- Marc Deslauriers <email address hidden> Wed, 23 Nov 2011 09:27:14 -0500

[Launchpad Janitor (janitor)]

This bug was fixed in the package update-notifier - 0.105ubuntu1.1

---------------
update-notifier (0.105ubuntu1.1) maverick-security; urgency=low

  * SECURITY UPDATE: hotfix for arbitrary code execution via directory
    traversal in update-manager on iso media (LP: #881548)
    - data/cddistupgrader: patch update-manager that is pulled off an
      upgrade cd.
    - debian/update-manager-downloader-fix2.diff: hotfix to verify
      signature before unpacking the tarball in
      UpdateManager/Core/DistUpgradeFetcherCore.py.
    - debian/update-notifier-common.*: ship new hotfix in package.
    - CVE-2011-3152
 -- Marc Deslauriers <email address hidden> Thu, 24 Nov 2011 12:58:59 -0500

[Launchpad Janitor (janitor)]

This bug was fixed in the package update-manager - 1:0.142.23.1

---------------
update-manager (1:0.142.23.1) maverick-security; urgency=low

  * SECURITY UPDATE: arbitrary code execution via directory traversal
    (LP: #881548)
    - UpdateManager/Core/DistUpgradeFetcherCore.py: verify signature before
      unpacking the tarball.
    - CVE-2011-3152
  * SECURITY UPDATE: information leak via insecure temp file (LP: #881541)
    - DistUpgrade/DistUpgradeViewKDE.py: use mkstemp instead of mktemp.
    - CVE-2011-3154
 -- Marc Deslauriers <email address hidden> Wed, 23 Nov 2011 09:29:26 -0500

[Launchpad Janitor (janitor)]

This bug was fixed in the package update-notifier - 0.99.3ubuntu0.1

---------------
update-notifier (0.99.3ubuntu0.1) lucid-security; urgency=low

  * SECURITY UPDATE: hotfix for arbitrary code execution via directory
    traversal in update-manager on iso media (LP: #881548)
    - data/cddistupgrader: patch update-manager that is pulled off an
      upgrade cd.
    - debian/update-manager-downloader-fix2.diff: hotfix to verify
      signature before unpacking the tarball in
      UpdateManager/Core/DistUpgradeFetcherCore.py.
    - debian/update-notifier-common.*: ship new hotfix in package.
    - CVE-2011-3152
 -- Marc Deslauriers <email address hidden> Thu, 24 Nov 2011 13:02:45 -0500

[Launchpad Janitor (janitor)]

This bug was fixed in the package update-manager - 1:0.134.11.1

---------------
update-manager (1:0.134.11.1) lucid-security; urgency=low

  * SECURITY UPDATE: arbitrary code execution via directory traversal
    (LP: #881548)
    - UpdateManager/Core/DistUpgradeFetcherCore.py: verify signature before
      unpacking the tarball.
    - CVE-2011-3152
  * SECURITY UPDATE: information leak via insecure temp file (LP: #881541)
    - DistUpgrade/DistUpgradeViewKDE.py: use mkstemp instead of mktemp.
    - CVE-2011-3154
 -- Marc Deslauriers <email address hidden> Wed, 23 Nov 2011 09:31:48 -0500

[Launchpad Janitor (janitor)]

This bug was fixed in the package update-manager - 1:0.87.31.1

---------------
update-manager (1:0.87.31.1) hardy-security; urgency=low

  * SECURITY UPDATE: arbitrary code execution via directory traversal
    (LP: #881548)
    - UpdateManager/Core/DistUpgradeFetcherCore.py: verify signature before
      unpacking the tarball.
    - CVE-2011-3152
  * SECURITY UPDATE: information leak via insecure temp file (LP: #881541)
    - DistUpgrade/DistUpgradeViewKDE.py: use mkstemp instead of mktemp.
    - CVE-2011-3154
 -- Marc Deslauriers <email address hidden> Wed, 23 Nov 2011 09:58:49 -0500

[Ubuntu Foundations Team Bug Bot (crichton)]

The attachment "obsolete patch" of this bug report has been identified as being a patch. The ubuntu-reviewers team has been subscribed to the bug report so that they can review the patch. In the event that this is in fact not a patch you can resolve this situation by removing the tag 'patch' from the bug report and editing the attachment so that it is not flagged as a patch. Additionally, if you are member of the ubuntu-sponsors please also unsubscribe the team from this bug report.

[This is an automated message performed by a Launchpad user owned by Brian Murray. Please contact him regarding any issues with the action taken in this bug report.]

[Launchpad Janitor (janitor)]

This bug was fixed in the package update-manager - 1:0.154.5

---------------
update-manager (1:0.154.5) precise; urgency=low

  [ Nicholas Skaggs ]
  * lp:~nskaggs/update-manager/fix-for-702418:
    - Removed gnome-power-manager dbus interface completely and
      only use freedesktop interface.
      Thanks to Nicholas Skaggs (LP: #702418)

  [ Gabor Kelemen ]
  * Replace gettext.install() with bindtextdomain() calls.
    Work around crash in OptionParser when displaying
    localized --help text, to not regress on bug LP: #557804
  * Extract strings for translation from u-m-t and u-s-s executables

  [ Marc Deslauriers ]
  * SECURITY UPDATE: arbitrary code execution via directory traversal
    (LP: #881548)
    - UpdateManager/Core/DistUpgradeFetcherCore.py: verify signature before
      unpacking the tarball.
    - CVE-2011-3152
  * SECURITY UPDATE: information leak via insecure temp file (LP: #881541)
    - DistUpgrade/DistUpgradeViewKDE.py: use mkstemp instead of mktemp.
    - CVE-2011-3154

  [ Michael Vogt ]
  * UpdateManager/UpdateManager.py:
    - ensure that the origin headers state of "select all/dselect all"
      is consistent
 -- Michael Vogt <email address hidden> Tue, 29 Nov 2011 09:58:15 +0100