New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 4 users
Status: Verified
Owner:
Email to this user bounced
Closed: Mar 2014
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 0
Type: Bug-Security

Blocking:
issue 351788



Sign in to add a comment
Security: dump_vpd_log can be tricked into creating a file (or corrupt non-regular file)
Project Member Reported by keescook@chromium.org, Feb 14 2014 Back to list
If an attack has managed to gain root on a running system, the following flaw would allow the creation of a file at boot time. If there is a sensitive flag file in a tmpfs, this could be used for persistence.

remove /home/chronos/.oobe_completed
        /usr/sbin/dump_vpd_log runs

make sure these exists:
        /mnt/stateful_partition/unencrypted/cache/vpd/full-v2.txt
        /mnt/stateful_partition/unencrypted/cache/vpd/filtered.txt

make sure these are symlinks (to anywhere, doesn't matter):
        /var/cache/vpd/full-v2.txt
        /var/cache/echo/vpd_echo.txt
        /var/log/vpd_2.0.txt

make sure this isn't a regular file, and symlink this to what you want to create (via full-v2.txt sed):
        /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt

this happens (note mkdir, sed, chown are unchecked):
        mkdir -p /mnt/stateful_partition/unencrypted/cache/vpd
        chown root:root /mnt/stateful_partition/unencrypted/cache/vpd || exit 1
        chmod go-stwx "$1" /mnt/stateful_partition/unencrypted/cache/vpd || exit 1
        chmod ugo+x "$1" /mnt/stateful_partition/unencrypted/cache/vpd || exit 1

        (generate_echo_codes)
        mkdir -p /mnt/stateful_partition/unencrypted/cache/vpd/echo
        mkdir -p /var/cache/echo
        sed -e (see below) < /mnt/stateful_partition/unencrypted/cache/vpd/full-v2.txt \
                > /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt
        chown -R chronos:chronos /mnt/stateful_partition/unencrypted/cache/vpd/echo
        chown -R chronos:chronos /var/cache/echo

The "chown" is safe because of "-R", it'll only target the symlink and stop, it doesn't recurse (e.g. -H would be needed to make it dangerous).

The sed call would allow the file contents to have lines that start with from full-v2.txt:

    "ubind_attribute"=
    "gbind_attribute"=

Everything else is ignored.

As long as /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt isn't a regular file, it will get written to:

    if [ ! -f ${ECHO_COUPON_FILE} ]; then
      generate_echo_codes
    fi

I haven't found anything in any of the tmpfs mounts to attack yet, though, so this appears to presently be a non-issue. We should still fix dump_vpd_log to be much more defensive about its actions, though.

 
(sorry for typos, the "$1" things shouldn't be there in my above walk-through)
Comment 2 by jln@chromium.org, Feb 14 2014
Labels: Security_Severity-None
Status: Available
Agreed, this looks bad and we should make this code more robust. (I'm using Severity-None purely for triage reasons).

I don't know anything about this, so here are my dumb questions:

- When one removes "/home/chronos/.oobe_completed", when does /usr/sbin/dump_vpd_log run?
- Of course the empty file is created much before a user's home directory gets mounted, correct? There would be no way to use this attack to create an empty file somewhere in Chrome profile?

"this happens (note mkdir, sed, chown are unchecked):" -> Could you give a link to the code to make perusing easier?
Yes, more context, this is from my review of chromeos_startup (~/trunk/src/platform/init), which is what calls dump_vpd_log:

https://chromium.googlesource.com/chromiumos/platform/vpd/+/master/util/dump_vpd_log

This "attack" is unique in that it happens before most of the system has started, which would allow manipulation of tmpfs files. For an attacker to perform this attack, they'd already need full root access to the stateful filesystem, so using this to manipulate stateful doesn't make sense.

Cc: gauravsh@chromium.org wad@chromium.org sumit@chromium.org
Labels: -Restrict-View-SecurityTeam Restrict-View-Google
Cc: yjlou@chromium.org
Blocking: chromium:351788
Labels: -Pri-2 -Security_Severity-None Pri-0 Security_Severity-Critical
Labels: Security_Impact-Beta Security_Impact-Stable
Comment 11 by wad@chromium.org, Mar 12 2014
Owner: yjlou@chromium.org
Didn't see your update.  An alternative CL is here:
  https://chromium-review.googlesource.com/#/c/189433/

It adds a few more matching checks.  Any thoughts?

It is still incomplete for this whole bug.  Really this script needs to be refactored :/

Bouncing to you yjlou@ for review feedback and testing.  Does that work?

Thanks!
Status: Assigned
And mine... let's see if we can combine! :) https://chromium-review.googlesource.com/#/c/189685/
I'd like to see the entire thing run under "set -e", which will clean up all the || exit 1 additions.

Comment 15 by yjlou@chromium.org, Mar 12 2014
Merged mine CL to Will's and some additional fixed. Merging Keescook's. Will upload again. Also doing test.
Comment 16 by yjlou@chromium.org, Mar 12 2014
https://chromium-review.googlesource.com/#/c/189433/ merged Will's and Keescook's CLs. Also set -e. Doing testing.
Project Member Comment 17 by bugdroid1@chromium.org, Mar 13 2014
Labels: M-34
Project  : chromiumos/platform/vpd
Branch   : release-R34-5500.B
Author   : Will Drewry <wad@chromium.org>
Committer: Yung-chieh Lo <yjlou@chromium.org>
Commit   : 5f1371ec23712d8ff318d1443765a21800aa10c7

Code-Review  0 : Yung-chieh Lo
Code-Review  +2: Jorge Lucangeli Obes
Commit-Queue 0 : Jorge Lucangeli Obes
Commit-Queue +1: Yung-chieh Lo
Verified     0 : Jorge Lucangeli Obes
Verified     +1: Yung-chieh Lo
Commit Queue   : Chumped
Change-Id      : Id9aa96b28ad52223085216f18bf1d663db7ca846
Reviewed-at    : https://chromium-review.googlesource.com/189744

dump_vpd_log: clean up existence checks (release-R34-5500.B)

Clean up some failure modes for dump_vpd_log. In practice,
this script needs to run set -e as well.  However, rather
than refactor, it would make sense to split out the behavior
either into independent shell scripts (with shunit tests)
or to integrate into a better tested binary (C++).

BUG= chromium:344051 
TEST=Verified on Pixel.
* Symlink to a fake file
rm -f /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt
ln -s /tmp/fake /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt
rm /home/chronos/.oobe_completed && /usr/sbin/dump_vpd_log
* Expect /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt
*   is recovered as a plain text and content is correct.
* Expect /var/cache/echo/vpd_echo.txt points to
*   /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt.
* Expect file and directory permissions are correct.

* On HP Chromebook 11
dump_vpd_log --clean
ln -s /tmp/fake /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt
rm /home/chronos/.oobe_completed && reboot machine

* Verified VPD works as usual.
* Verified with the file timestamp that cached is not generated for
*   reboot.

Original-Change-Id:

util/dump_vpd_log
Project Member Comment 18 by bugdroid1@chromium.org, Mar 13 2014
Labels: M-33
Project  : chromiumos/platform/vpd
Branch   : release-R33-5116.B
Author   : Will Drewry <wad@chromium.org>
Committer: Yung-chieh Lo <yjlou@chromium.org>
Commit   : 57aecd8484a47c39f17ea04e463c8d56a9485611

Code-Review  0 : Yung-chieh Lo
Code-Review  +2: Jorge Lucangeli Obes
Commit-Queue 0 : Jorge Lucangeli Obes
Commit-Queue +1: Yung-chieh Lo
Verified     0 : Jorge Lucangeli Obes
Verified     +1: Yung-chieh Lo
Commit Queue   : Chumped
Change-Id      : I42ddae7c90c19e36e06aea3282b8864ebf4d9218
Reviewed-at    : https://chromium-review.googlesource.com/189770

dump_vpd_log: clean up existence checks (release-R33-5116.B)

Clean up some failure modes for dump_vpd_log. In practice,
this script needs to run set -e as well.  However, rather
than refactor, it would make sense to split out the behavior
either into independent shell scripts (with shunit tests)
or to integrate into a better tested binary (C++).

BUG= chromium:344051 
TEST=Verified on Pixel.
* Symlink to a fake file
rm -f /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt
ln -s /tmp/fake /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt
rm /home/chronos/.oobe_completed && /usr/sbin/dump_vpd_log
* Expect /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt
*   is recovered as a plain text and content is correct.
* Expect /var/cache/echo/vpd_echo.txt points to
*   /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt.
* Expect file and directory permissions are correct.

* On HP Chromebook 11
dump_vpd_log --clean
ln -s /tmp/fake /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt
rm /home/chronos/.oobe_completed && reboot machine

* Verified VPD works as usual.
* Verified with the file timestamp that cached is not generated for
*   reboot.

Original-Change-Id:

util/dump_vpd_log
can we mark this as Fixed now ?
Labels: Release-2-M33
Status: Fixed
Labels: -Release-2-M33 Release-3-M33
Project Member Comment 22 by bugdroid1@chromium.org, Mar 13 2014
Project  : chromiumos/platform/vpd
Branch   : stabilize-5116.115.B
Author   : Will Drewry <wad@chromium.org>
Committer: Jorge Lucangeli Obes <jorgelo@chromium.org>
Commit   : 87698ed70d2f0150620a5c1549d45d2d3798992a

Code-Review  0 : Yung-chieh Lo
Code-Review  +2: Jorge Lucangeli Obes
Commit-Queue 0 : Jorge Lucangeli Obes
Commit-Queue +1: Yung-chieh Lo
Verified     0 : Jorge Lucangeli Obes
Verified     +1: Yung-chieh Lo
Commit Queue   : Chumped
Change-Id      : I35cf321784cbe6a16e182030b7ef70af3a6b7e86
Reviewed-at    : https://chromium-review.googlesource.com/189755

dump_vpd_log: clean up existence checks (stabilize-5116.115.B)

Clean up some failure modes for dump_vpd_log. In practice,
this script needs to run set -e as well.  However, rather
than refactor, it would make sense to split out the behavior
either into independent shell scripts (with shunit tests)
or to integrate into a better tested binary (C++).

BUG= chromium:344051 
TEST=Verified on Pixel.
* Symlink to a fake file
rm -f /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt
ln -s /tmp/fake /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt
rm /home/chronos/.oobe_completed && /usr/sbin/dump_vpd_log
* Expect /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt
*   is recovered as a plain text and content is correct.
* Expect /var/cache/echo/vpd_echo.txt points to
*   /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt.
* Expect file and directory permissions are correct.

* On HP Chromebook 11
dump_vpd_log --clean
ln -s /tmp/fake /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt
rm /home/chronos/.oobe_completed && reboot machine

* Verified VPD works as usual.
* Verified with the file timestamp that cached is not generated for
*   reboot.

Original-Change-Id:

util/dump_vpd_log
Project Member Comment 23 by bugdroid1@chromium.org, Mar 13 2014
Project  : chromiumos/platform/vpd
Branch   : master
Author   : Will Drewry <wad@chromium.org>
Committer: Jorge Lucangeli Obes <jorgelo@chromium.org>
Commit   : bf65516560270b212fed4d9b3052e3905d66931a

Code-Review  0 : Gaurav Shah, chrome-internal-fetch
Code-Review  +1: Jorge Lucangeli Obes, Yung-chieh Lo
Code-Review  +2: Kees Cook
Commit-Queue 0 : Gaurav Shah, Jorge Lucangeli Obes, Kees Cook, chrome-internal-fetch
Commit-Queue +1: Yung-chieh Lo
Verified     0 : Gaurav Shah, Jorge Lucangeli Obes, Kees Cook, chrome-internal-fetch
Verified     +1: Yung-chieh Lo
Commit Queue   : Chumped
Change-Id      : Ib4d0aa51feebf5ae95050c0a043c29d8b7212ff4
Reviewed-at    : https://chromium-review.googlesource.com/189433

dump_vpd_log: clean up existence checks

Clean up some failure modes for dump_vpd_log. In practice,
this script needs to run set -e as well.  However, rather
than refactor, it would make sense to split out the behavior
either into independent shell scripts (with shunit tests)
or to integrate into a better tested binary (C++).

BUG= chromium:344051 
TEST=Verified on Pixel.
* Symlink to a fake file
rm -f /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt
ln -s /tmp/fake /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt
rm /home/chronos/.oobe_completed && /usr/sbin/dump_vpd_log
* Expect /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt
*   is recovered as a plain text and content is correct.
* Expect /var/cache/echo/vpd_echo.txt points to
*   /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt.
* Expect file and directory permissions are correct.

* On HP Chromebook 11
dump_vpd_log --clean
ln -s /tmp/fake /mnt/stateful_partition/unencrypted/cache/vpd/echo/vpd_echo.txt
rm /home/chronos/.oobe_completed && reboot machine

* Verified VPD works as usual.
* Verified with the file timestamp that cached is not generated for
*   reboot.

util/dump_vpd_log
Cc: deepakg@chromium.org
Comment 25 by ka...@chromium.org, Mar 13 2014
Status: Verified
Tested on 5116.115.[2,3] link, daisy by:
Following test notes in CL
Switching vpd and verifying UX for other lang/kb locale settings.
Labels: CVE-2014-1708
Comment 27 by k...@google.com, Mar 14 2014
Cc: kamakshi@chromium.org
Labels: -Restrict-View-Google
Removing R-V-G.
Project Member Comment 29 by sheriffbot@chromium.org, Mar 22 2016
Labels: -security_impact-beta
Project Member Comment 30 by sheriffbot@chromium.org, Oct 1 2016
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
Project Member Comment 31 by sheriffbot@chromium.org, Oct 2 2016
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
Project Member Comment 32 by sheriffbot@chromium.org, Oct 2 2016
Labels: Restrict-View-SecurityNotify
Labels: allpublic
Project Member Comment 34 by sheriffbot@chromium.org, Oct 3 2016
Labels: -Restrict-View-SecurityNotify
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
Sign in to add a comment