New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 734559 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: ChromeOS PPD Import Check Buffer Overflow

Reported by r...@rorym.cnamara.com, Jun 19 2017

Issue description

When importing a PPD file into the native CUPS server, the 'cupstestppd' binary is used to check the validity of the PPD file before importing.

I found this using the org.chromium.debugd.CupsAddPrinter dbus endpoint, but it is also available using the addCupsPrinter 'chrome.send' function on the chrome://md-settings page.

There is a buffer overflow when reading the PSVersion value, which, if FORTIFY_SOURCE was not present, would allow a maliciously crafted PPD file to overwrite the errors uint, setting it to a negative value, such that any future errors resulted in a final exit code of 0, effectively bypassing the use of "cupstestppd" as a security boundary.

The malicious PPD would then be imported which, for example, could execute any command using the 'cupsFilter' functionality in the PPD. Commands are executed as a low privileged user inside a seccomp sandbox (child of 'cupsd', not 'cupstestppd').

The relevant code, including the execution of 'cupstestppd' can be found in 'debugd/src/cups_tool.cc' [1]

Since FORTIFY_SOURCE is present on ChromeOS, the 'errors' uint is above the overflowed buffer on the stack, so it is not overwritten. Without FORTIFY_SOURCE this uint may be below the buffer, allowing for direct overwriting. ASLR also blocks standard buffer overflow exploitation methods, since this is a short lived process and there are no known ASLR leaks.

I've got a patch for this bug merged upstream, [2], but given the use of this binary on ChromeOS, and given that native printing was recently enabled in Stable, I thought it was worth submitting here also.

[1] https://chromium.googlesource.com/chromiumos/platform2/+/master/debugd/src/cups_tool.cc#248
[2] https://github.com/apple/cups/pull/4996
 
Components: Internals>Printing>CUPS
Labels: OS-Chrome
Thanks for the great report!

Comment 2 by est...@chromium.org, Jun 20 2017

Cc: jorgelo@chromium.org wad@chromium.org
Labels: Needs-Feedback
Thanks for the report. Am I understanding correctly that this is not exploitable on ChromeOS currently?
That is correct.

The Stack Canary stops classic buffer overflow exploitation, and the stack reordering due to FORTIFY_SOURCE blocks application specific attacks discussed above (overwriting 'uint errors').

It is possible to reliably crash cupstestppd, but the exit code is non-zero, meaning the ChromeOS code does not consider it a successful execution.
Project Member

Comment 4 by sheriffbot@chromium.org, Jun 20 2017

Cc: est...@chromium.org
Labels: -Needs-Feedback
Thank you for providing more feedback. Adding requester "estark@chromium.org" to the cc list and removing "Needs-Feedback" label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Owner: adlr@chromium.org
Status: Assigned (was: Unconfirmed)
Over to ADLR for routing. We should fix even if this is not currently exploitable.

Comment 6 by mmoroz@chromium.org, Jun 21 2017

Labels: Security_Severity-High Security_Impact-Stable Pri-1
Assigning High severity as the issue "can" lead to RCE within the sandbox.
Andrew, who can take this one?
Project Member

Comment 8 by sheriffbot@chromium.org, Jun 22 2017

Labels: M-59
Project Member

Comment 9 by sheriffbot@chromium.org, Jul 4 2017

adlr: Uh oh! This issue still open and hasn't been updated in the last 14 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Ping?
Project Member

Comment 11 by sheriffbot@chromium.org, Jul 19 2017

adlr: Uh oh! This issue still open and hasn't been updated in the last 29 days. This is a serious vulnerability, and we want to ensure that there's progress. Could you please leave an update with the current status and any potential blockers?

If you're not the right owner for this issue, could you please remove yourself as soon as possible or help us find the right one?

If the issue is fixed or you can't reproduce it, please close the bug. If you've started working on a fix, please set the status to Started.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Hey Andrew, can you route this please?
Cc: skau@chromium.org

Comment 14 by adlr@chromium.org, Jul 21 2017

Cc: -skau@chromium.org
Owner: skau@chromium.org
Sorry for the delay. As you can likely tell I'm suffering from email overload.

I think we are also immune from this specific hack b/c of this patch that we already have: src/third_party/chromiumos-overlay/net-print/cups/files/cups-2.1.4-strict-filters.patch

That said, Sean, can you take this? If you're busy I can dig in next week.
Cc: adlr@chromium.org

Comment 16 by skau@chromium.org, Jul 21 2017

Status: Started (was: Assigned)
I'll merge the patch from upstream.  The patch that adlr mentioned won't protect us as errors can be set to an appropriate negative number.
Project Member

Comment 17 by bugdroid1@chromium.org, Jul 22 2017

Comment 18 by skau@chromium.org, Jul 24 2017

Labels: Merge-Request-61 Merge-Request-60
Request merge to beta and dev.

jorgelo@: Is this severe enough to merge back to 59?  My instincts say no.

Comment 19 by skau@chromium.org, Jul 24 2017

Cc: keta...@chromium.org josa...@chromium.org
Project Member

Comment 20 by sheriffbot@chromium.org, Jul 24 2017

Labels: -Merge-Request-60 Hotlist-Merge-Review Merge-Review-60
This bug requires manual review: We are only 0 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
No need to merge to M59, Josafat mentioned that there are no more M59 releases planned.
Project Member

Comment 22 by sheriffbot@chromium.org, Jul 25 2017

Status: Fixed (was: Started)
Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 23 by sheriffbot@chromium.org, Jul 25 2017

Labels: -Merge-Request-61 Hotlist-Merge-Approved Merge-Approved-61
Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 24 by bugdroid1@chromium.org, Jul 26 2017

Labels: merge-merged-release-R61-9765.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/7220be32c0d1873806e6ea4e0d4d2a2307b4b238

commit 7220be32c0d1873806e6ea4e0d4d2a2307b4b238
Author: Sean Kau <skau@chromium.org>
Date: Wed Jul 26 00:11:26 2017

net-print/cups: Cherry pick patch from upstream for cupstestppd.

BUG= chromium:734559 
TEST=Verified we still print

Change-Id: I12ac6578633395299d892b97c6b78437a6161fe0
Previous-Reviewed-on: https://chromium-review.googlesource.com/581735
(cherry picked from commit f374b441e2da811fa4bfedcd27601d50f97b7c24)
Reviewed-on: https://chromium-review.googlesource.com/585552
Tested-by: Sean Kau <skau@chromium.org>
Trybot-Ready: Sean Kau <skau@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Commit-Queue: Sean Kau <skau@chromium.org>

[add] https://crrev.com/7220be32c0d1873806e6ea4e0d4d2a2307b4b238/net-print/cups/files/cups-2.1.4-limit-PSVersion-sscanf.patch
[modify] https://crrev.com/7220be32c0d1873806e6ea4e0d4d2a2307b4b238/net-print/cups/cups-2.1.4.ebuild
[rename] https://crrev.com/7220be32c0d1873806e6ea4e0d4d2a2307b4b238/net-print/cups/cups-2.1.4-r22.ebuild

Project Member

Comment 25 by sheriffbot@chromium.org, Jul 26 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -M-59 -Merge-Review-60 M-60 Merge-Approved-60
Project Member

Comment 27 by bugdroid1@chromium.org, Jul 26 2017

Labels: merge-merged-release-R60-9592.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/799f2bd663bbc99f3bde790a8136e37aa0a2a2cd

commit 799f2bd663bbc99f3bde790a8136e37aa0a2a2cd
Author: Sean Kau <skau@chromium.org>
Date: Wed Jul 26 18:28:37 2017

net-print/cups: Cherry pick patch from upstream for cupstestppd.

BUG= chromium:734559 
TEST=Verified we still print

Change-Id: I12ac6578633395299d892b97c6b78437a6161fe0
Reviewed-on: https://chromium-review.googlesource.com/581735
Commit-Ready: Sean Kau <skau@chromium.org>
Tested-by: Sean Kau <skau@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/587287
Trybot-Ready: Sean Kau <skau@chromium.org>
Commit-Queue: Sean Kau <skau@chromium.org>

[add] https://crrev.com/799f2bd663bbc99f3bde790a8136e37aa0a2a2cd/net-print/cups/files/cups-2.1.4-limit-PSVersion-sscanf.patch
[modify] https://crrev.com/799f2bd663bbc99f3bde790a8136e37aa0a2a2cd/net-print/cups/cups-2.1.4.ebuild
[add] https://crrev.com/799f2bd663bbc99f3bde790a8136e37aa0a2a2cd/net-print/cups/cups-2.1.4-r22.ebuild

Labels: reward-topanel
Cc: -adlr@chromium.org mmoroz@chromium.org
Labels: -Type-Bug-Security -Hotlist-Merge-Review -reward-topanel -Security_Impact-Stable -Security_Severity-High reward-0 Type-Bug
Changing to type bug as this does seem to be a DOS issue rather than security.  mmoroz@ - mind confirming?
I would rather treat this as a security issue, as FORTIFY_SOURCE is yet another mitigation making exploitation harder, but not impossible.
Labels: -Type-Bug -reward-0 Security_Impact-Stable reward-topanel Security_Severity-High Type-Bug-Security
As per severity guidelines, we may lower the severity if there is a mitigation in place: https://www.chromium.org/developers/severity-guidelines

Since cupstestppd is a daemon, I guess it could have a Critical severity, but in given circumstances it is High. If I'm wrong, could anyone from ChromeOS security please fix me? :)

Changing labels back to security.
cupstestppd is not a daemon, it is a short lived process executed to verify the PPD file on import, and the exit code is checked by debugd before the PPD is sent to cups
Labels: -Security_Severity-High Security_Severity-Medium
Ok, then it could be High, but is Medium in that case. Thank you rory@ for the great report and for clarifications! :)
Labels: -reward-topanel reward-unpaid reward-1000
*** Boilerplate reminders! ***
Please do NOT publicly disclose details until a fix has been released to all our users. Early public disclosure may cancel the provisional reward. Also, please be considerate about disclosure when the bug affects a core library that may be used by other products. Please do NOT share this information with third parties who are not directly involved in fixing the bug. Doing so may cancel the provisional reward. Please be honest if you have already disclosed anything publicly or to third parties. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an eligible charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing.
*********************************
Congratulations! The VRP panel decided to award $1,000 for this report! A member of our finance team will be in touch to arrange payment.
Thanks!
Labels: -reward-unpaid reward-inprocess
Project Member

Comment 38 by sheriffbot@chromium.org, Nov 1 2017

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 39 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Sign in to add a comment