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

Bug #881548 reported by David
262
This bug affects 1 person
Affects Status Importance Assigned to Milestone
update-manager (Ubuntu)
Critical
Michael Vogt
Hardy
High
Marc Deslauriers
Lucid
High
Marc Deslauriers
Maverick
High
Marc Deslauriers
Natty
High
Marc Deslauriers
Oneiric
High
Marc Deslauriers
Precise
Critical
Michael Vogt
update-notifier (Ubuntu)
High
Unassigned
Hardy
High
Marc Deslauriers
Lucid
High
Marc Deslauriers
Maverick
High
Marc Deslauriers
Natty
High
Marc Deslauriers
Oneiric
High
Marc Deslauriers
Precise
High
Unassigned

Bug Description

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

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

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

Revision history for this message
Michael Vogt (mvo) wrote :

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.

Changed in update-manager (Ubuntu):
assignee: nobody → Michael Vogt (mvo)
importance: Undecided → Critical
status: New → In Progress
Revision history for this message
Michael Vogt (mvo) wrote :
Revision history for this message
Michael Vogt (mvo) wrote :

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.

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

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.

Revision history for this message
David (d--) wrote :

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.

Revision history for this message
Michael Vogt (mvo) wrote :

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.

Revision history for this message
David (d--) wrote :

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 :-) ).

Revision history for this message
David (d--) wrote :

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

Revision history for this message
Michael Vogt (mvo) wrote :

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.

Revision history for this message
David (d--) wrote :

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).

Revision history for this message
David (d--) wrote :

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

Revision history for this message
Marc Deslauriers (mdeslaur) wrote :

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

Revision history for this message
Michael Vogt (mvo) wrote :

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.

Revision history for this message
Michael Vogt (mvo) wrote :

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

Changed in update-manager (Ubuntu Hardy):
status: New → Confirmed
Changed in update-manager (Ubuntu Lucid):
status: New → Confirmed
Changed in update-manager (Ubuntu Maverick):
status: New → Confirmed
Changed in update-manager (Ubuntu Natty):
status: New → Confirmed
Changed in update-manager (Ubuntu Oneiric):
status: New → Confirmed
Changed in update-manager (Ubuntu Hardy):
assignee: nobody → Marc Deslauriers (mdeslaur)
Changed in update-manager (Ubuntu Lucid):
assignee: nobody → Marc Deslauriers (mdeslaur)
Changed in update-manager (Ubuntu Natty):
assignee: nobody → Marc Deslauriers (mdeslaur)
Changed in update-manager (Ubuntu Maverick):
assignee: nobody → Marc Deslauriers (mdeslaur)
Changed in update-manager (Ubuntu Oneiric):
assignee: nobody → Marc Deslauriers (mdeslaur)
Changed in update-manager (Ubuntu Hardy):
importance: Undecided → High
Changed in update-manager (Ubuntu Lucid):
importance: Undecided → High
Changed in update-manager (Ubuntu Maverick):
importance: Undecided → High
Changed in update-manager (Ubuntu Natty):
importance: Undecided → High
Changed in update-manager (Ubuntu Oneiric):
importance: Undecided → High
Changed in update-notifier (Ubuntu Hardy):
assignee: nobody → Marc Deslauriers (mdeslaur)
importance: Undecided → High
status: New → Confirmed
Changed in update-notifier (Ubuntu Lucid):
assignee: nobody → Marc Deslauriers (mdeslaur)
importance: Undecided → High
status: New → Confirmed
Changed in update-notifier (Ubuntu Maverick):
assignee: nobody → Marc Deslauriers (mdeslaur)
importance: Undecided → High
status: New → Confirmed
Changed in update-notifier (Ubuntu Natty):
assignee: nobody → Marc Deslauriers (mdeslaur)
importance: Undecided → High
status: New → Confirmed
Changed in update-notifier (Ubuntu Oneiric):
assignee: nobody → Marc Deslauriers (mdeslaur)
importance: Undecided → High
status: New → Confirmed
Changed in update-notifier (Ubuntu Precise):
importance: Undecided → High
status: New → Confirmed
Changed in update-notifier (Ubuntu Oneiric):
status: Confirmed → Invalid
Changed in update-notifier (Ubuntu Precise):
status: Confirmed → Invalid
Changed in update-notifier (Ubuntu Hardy):
status: Confirmed → Won't Fix
Revision history for this message
Launchpad Janitor (janitor) wrote :

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

Revision history for this message
Launchpad Janitor (janitor) wrote :

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

Revision history for this message
Launchpad Janitor (janitor) wrote :

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

Revision history for this message
Launchpad Janitor (janitor) wrote :

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

Revision history for this message
Launchpad Janitor (janitor) wrote :

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

Revision history for this message
Launchpad Janitor (janitor) wrote :

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

Revision history for this message
Launchpad Janitor (janitor) wrote :

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

Revision history for this message
Launchpad Janitor (janitor) wrote :

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

Changed in update-manager (Ubuntu Hardy):
status: Confirmed → Fix Released
Changed in update-manager (Ubuntu Lucid):
status: Confirmed → Fix Released
Changed in update-manager (Ubuntu Maverick):
status: Confirmed → Fix Released
Changed in update-manager (Ubuntu Natty):
status: Confirmed → Fix Released
Changed in update-manager (Ubuntu Oneiric):
status: Confirmed → Fix Released
Changed in update-notifier (Ubuntu Lucid):
status: Confirmed → Fix Released
Changed in update-notifier (Ubuntu Maverick):
status: Confirmed → Fix Released
Changed in update-notifier (Ubuntu Natty):
status: Confirmed → Fix Released
visibility: private → public
Revision history for this message
Ubuntu Foundations Team Bug Bot (crichton) wrote :

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.]

tags: added: patch
Revision history for this message
Launchpad Janitor (janitor) wrote :

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

Changed in update-manager (Ubuntu Precise):
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Other bug subscribers