Arbitrary code execution through crafted CrashDB or Package/Source fields in .crash files

@security team: Can you please assign a CVE?

I'm on the core sprint this week, and next week I have two work days left. I'll look into this on Monday and attach patches here for review, but I most probably won't be able to do all the backports to stable releases, so I'll need some help with that.

I have attached a minimal Apport crash file which results in code execution when opened.

I've also uploaded a private video demonstrating this bug at https://vimeo.com/194867494 (password: apportbugpoc)

Proposed patch (including tests) for the code execution via crafted CrashDB: fields, against current trunk.

I will now look into the other aspect (arbitrary hook execution via crafted Package:/Source: fields). I think this deserves a separate CVE number.

Subscribing Brian, who is taking over Apport maintenance, just in case this takes longer. Brian, this issue has not been disclosed yet.

The previous patch contained an additional check for the data type which is unnecessary, I just forgot to remove it again (thinko, problem_report.py already enforces this).

The patch for the CrashDB bug looks good to me

This Apport crash file exploits the Package name path traversal bug to load a Python hook file from the users Downloads directory (~/Downloads/hook.py). It would be straight forward to trick a user on Chromium as it automatically downloads files with prompt to the user's Downloads directory.

Proposed patch for the path traversal issue, including tests. This affects all apport versions back to precise. (The CrashDB: one only affects trusty and later, not precise).

Hi Martin, thank you very much for preparing patches, we'll take it from here!

The security team can't currently assign CVE numbers, so we can't assign numbers to this issue.

Should I apply for CVE numbers directly from MITRE or will the security team request them?

Please apply directly, sorry for the inconvenience.

There is a another risk where the `RespawnCommand` or `ProcCmdline` fields in a crash file are executed when a user clicks "Restart" from the apport-gtk prompt. This is not a vulnerability as it is the intended behaviour

However it may be safer to hide the restart option when opening a non-local crash file (when a .crash is opened directly from the Desktop). Instead the "Restart" button would only be shown when a generated crash file is opened by the `update-notifier` inotify watcher.

I've requested two CVE's from MITRE. I'll add the to the ticket when I receive a response.

> However it may be safer to hide the restart option when opening a non-local crash file

Agreed. update-notifier calls apport-* without any arguments, so it will iterate over /var/crash/. Clicking on a .crash file (in any directory) will call apport-* with that file name. So we'll only show "Restart" in the former.

This fixes the third aspect (arbitrary code execution on malicious .crash files through "Relaunch"). I think if the other two issues warrant a CVE, then this one does too.

This also affects all versions back to precise.

Martin: Thanks for the third patch. That looks like the right approach.

I have not yet received a response for my CVE requests to MITRE. Alternatively we can request CVE IDs from the oss-security mailing list. I don't think we need to wait for MITRE before releasing the updates.

I'll still be able to do computer stuff until next Tuesday (for landing in trunk and doing a new upstream release and zesty upload), if we want to publish by then. Otherwise, if it gets any later I really suggest to postpone this until start of January.

I'd like to see the the patches and and blog post released this week if possible. Please let me know if there is anything I can do to help.

I spoke with Debian earlier today and we do not need to coordinate with them for this issue as apport is only in experimental and they can update it after this issue is made public.

We (Ubuntu Security) are internally investigating if we could do a coordinated release date (CRD) of this week (taking security update prep and QA into account). We'll report back in a little while.

Martin, thanks for the third patch; I've updated the packages that Marc had worked on to incorporate them. The patch applied with little difficulty on all releases except for precise; I'm attaching the debdiff for that release for review.

Donncha: would a CRD of 22:00UTC December 14th (i.e. tomorrow) work for you?

Thanks.

Steve: Yes a CRD of 22:00 UTC today December 14th works for me.

Can you link to https://donncha.is/2016/12/compromising-ubuntu-desktop/ in the notification as it provides more information. That post will be published at 22:00 UTC. Thanks for you speed in getting this released.

OK, I'll prepare a new upstream release locally then and push bzr/publish it on 22:00 UTC today. Thanks!

MITRE has assigned CVE-2016-9949 for the CrashDB bug, and CVE-2016-9950 for the hook path traversal bug.

I haven't yet to receive a response to my follow up request for a CVE-ID for the "Relaunch" issue.

MITRE have issued CVE-2016-9951 for the "Relaunch" issue.

I already prepared the upstream and zesty release this afternoon, but I guess I'll re-do that now to add the CVE numbers. Thanks for requesting them!

FTR, everything is good to go on my side for new upstream release and tested zesty package (including CVEs), will unleash in 1.5 hours.

On Wed, Dec 14, 2016 at 07:17:15AM -0000, Donncha O Cearbhaill wrote:
> Steve: Yes a CRD of 22:00 UTC today December 14th works for me.
>
> Can you link to https://donncha.is/2016/12/compromising-ubuntu-desktop/
> in the notification as it provides more information. That post will be
> published at 22:00 UTC. Thanks for you speed in getting this released.

We don't include external references in the announcements themselves;
however, I'll ensure a reference to your blog post is included with
each of the CVE entries in the Ubuntu Security Team's CVE tracker,
now that Mitre has assigned CVEs. The CVE entries are linked from
the USN, so it'll still be findable for anyone who researches the
specific issues.

Thanks!
--
Steve Beattie
<email address hidden>
http://NxNW.org/~steve/

This bug was fixed in the package apport - 2.0.1-0ubuntu17.15

---------------
apport (2.0.1-0ubuntu17.15) precise-security; urgency=medium

  [ Marc Deslauriers ]
  * SECURITY UPDATE: path traversal vulnerability with hooks execution
    - Clean path in apport/report.py, added test to test/test_ui.py.
    - No CVE number
    - LP: #1648806

  [ Steve Beattie ]
  * SECURITY UPDATE: code execution via malicious crash files
    - Only offer restarting the application when processing a
      crash file in /var/crash in apport/ui.py, gtk/apport-gtk,
      and kde/apport-kde. Add testcases to test/test_ui.py,
      test/test_ui_gtk.py, and test_ui_kde.py.
    - No CVE number
    - LP: #1648806

 -- Marc Deslauriers <email address hidden> Mon, 12 Dec 2016 07:34:52 -0500

This bug was fixed in the package apport - 2.20.1-0ubuntu2.4

---------------
apport (2.20.1-0ubuntu2.4) xenial-security; urgency=medium

  [ Marc Deslauriers ]
  * SECURITY UPDATE: code execution via malicious crash files
    - Use ast.literal_eval in apport/ui.py, added test to test/test_ui.py.
    - No CVE number
    - LP: #1648806
  * SECURITY UPDATE: path traversal vulnerability with hooks execution
    - Clean path in apport/report.py, added test to test/test_ui.py.
    - No CVE number
    - LP: #1648806

  [ Steve Beattie ]
  * SECURITY UPDATE: code execution via malicious crash files
    - Only offer restarting the application when processing a
      crash file in /var/crash in apport/ui.py, gtk/apport-gtk,
      and kde/apport-kde. Add testcases to test/test_ui.py,
      test/test_ui_gtk.py, and test_ui_kde.py.
    - No CVE number
    - LP: #1648806

 -- Marc Deslauriers <email address hidden> Mon, 12 Dec 2016 07:26:36 -0500

New upstream release with the fixes: https://launchpad.net/apport/trunk/2.20.4

Note that Brian committed some changes to trunk in the last 1.5 hours, so we had some mid-air collection. I force-pushed trunk and will put back his commits on top.

This bug was fixed in the package apport - 2.14.1-0ubuntu3.23

---------------
apport (2.14.1-0ubuntu3.23) trusty-security; urgency=medium

  [ Marc Deslauriers ]
  * SECURITY UPDATE: code execution via malicious crash files
    - Use ast.literal_eval in apport/ui.py, added test to test/test_ui.py.
    - No CVE number
    - LP: #1648806
  * SECURITY UPDATE: path traversal vulnerability with hooks execution
    - Clean path in apport/report.py, added test to test/test_ui.py.
    - No CVE number
    - LP: #1648806

  [ Steve Beattie ]
  * SECURITY UPDATE: code execution via malicious crash files
    - Only offer restarting the application when processing a
      crash file in /var/crash in apport/ui.py, gtk/apport-gtk,
      and kde/apport-kde. Add testcases to test/test_ui.py,
      test/test_ui_gtk.py, and test_ui_kde.py.
    - No CVE number
    - LP: #1648806

 -- Marc Deslauriers <email address hidden> Mon, 12 Dec 2016 07:27:21 -0500

This bug was fixed in the package apport - 2.20.3-0ubuntu8.2

---------------
apport (2.20.3-0ubuntu8.2) yakkety-security; urgency=medium

  [ Marc Deslauriers ]
  * SECURITY UPDATE: code execution via malicious crash files
    - Use ast.literal_eval in apport/ui.py, added test to test/test_ui.py.
    - No CVE number
    - LP: #1648806
  * SECURITY UPDATE: path traversal vulnerability with hooks execution
    - Clean path in apport/report.py, added test to test/test_ui.py.
    - No CVE number
    - LP: #1648806

  [ Steve Beattie ]
  * SECURITY UPDATE: code execution via malicious crash files
    - Only offer restarting the application when processing a
      crash file in /var/crash in apport/ui.py, gtk/apport-gtk,
      and kde/apport-kde. Add testcases to test/test_ui.py,
      test/test_ui_gtk.py, and test_ui_kde.py.
    - No CVE number
    - LP: #1648806

 -- Marc Deslauriers <email address hidden> Tue, 13 Dec 2016 10:55:09 -0800

The attachment "proposed fix for CrashDB code execution" seems to be a patch. If it isn't, please remove the "patch" flag from the attachment, remove the "patch" tag, and if you are a member of the ~ubuntu-reviewers, unsubscribe the team.

[This is an automated message performed by a Launchpad user owned by ~brian-murray, for any issues please contact him.]

This bug was fixed in the package apport - 2.20.4-0ubuntu1

---------------
apport (2.20.4-0ubuntu1) zesty; urgency=medium

  * New upstream release:
    - SECURITY FIX: Restrict a report's CrashDB field to literals.
      Use ast.literal_eval() instead of the generic eval(), to prevent
      arbitrary code execution from malicious .crash files. A user could be
      tricked into opening a crash file whose CrashDB field contains an
      exec(), open(), or similar commands; this is fairly easy as we install a
      MIME handler for these. Thanks to Donncha O'Cearbhaill for discovering
      this! (CVE-2016-9949, LP: #1648806)
    - SECURITY FIX: Fix path traversal vulnerability with hooks execution.
      Ensure that Package: and SourcePackage: fields loaded from reports do
      not contain directories. Until now, an attacker could trick a user into
      opening a malicious .crash file containing "Package:
      ../../../../some/dir/foo" which would execute /some/dir/foo.py with
      arbitrary code. Thanks to Donncha O'Cearbhaill for discovering this!
      (CVE-2016-9950, LP: #1648806)
    - SECURITY FIX: apport-{gtk,kde}: Only offer "Relaunch" for recent
      /var/crash crashes.
      It only makes sense to offer relaunching for crashes that just happened
      and the apport UI got triggered on those. When opening a .crash file
      copied from somewhere else or after the crash happened, this is even
      actively dangerous as a malicious crash file can specify any arbitrary
      command to run. Thanks to Donncha O'Cearbhaill for discovering this!
      (CVE-2016-9951, LP: #1648806)
    - backends/packaging-apt-dpkg.py: provide a fallback method if using zgrep
      to search for a file in Contents.gz fails due to a lack of memory.
      Thanks Brian Murray.
    - bin/apport-retrace: When --core-file is used instead of loading the core
      file and adding it to the apport report just pass the file reference to
      gdb.
  * debian/control: Adjust Vcs-Bzr: for zesty branch.

 -- Martin Pitt <email address hidden> Wed, 14 Dec 2016 21:28:57 +0100

Question:

The release notes state: "Use ast.literal_eval() instead of the generic eval(), to prevent arbitrary code execution from malicious .crash files"

The change should be in ui.py in this revision:

http://bazaar.launchpad.net/~apport-hackers/apport/trunk/revision/3114

Just to be clear: How does "self.offer_restart = True" avoid generic "eval()" and use "ast.literal_eval()" instead?

Does this also mean that there are still situations where "eval()" is called? And why? This always leads to security issues, it's just a matter of time.

Thanks for fixing it quickly.

@benjaoming Looks like commit notes mixed up between 3114 and 3112.

The eval fix (CVE-2016-9949) is in 3112:

https://bazaar.launchpad.net/~apport-hackers/apport/trunk/revision/3112/

The patch in 3114 fixes CVE-2016-9951 (Relaunch code execution).

@Benjamin: Argh, I had to uncommit/recommit these three as the CVE numbers came in at the last minute, and apparently got the commit messages the wrong way around (meh @ not having rebase in bzr..) I did some surgery on the branch and the commit messages are correct now.

When I created the fixes I also verified that this was the only eval() in the entire source, there is none left now.