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

Issue 537205 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Email to this user bounced
Closed: Oct 2015
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: Crazy Linker on Android allows modification of Chrome APK without breaking signature

Reported by bednarsk...@gmail.com, Sep 29 2015

Issue description

VULNERABILITY DETAILS
Chrome on Android loads it's libraries using it's own fork of Android Crazy Linker, which parses Chrome APK file itself, bit differently than Android does. In particular when looking for end-of-central-directory-record signature (PK\5\6), Chrome's Crazy Linker starts searching at file size minus length of signature and Android starts searching at file size minus size of EocdRecord. This leads to Chrome-specific variant of Android "Master Key" bug.

VERSION
Chrome Version: 45.0.2454.94 stable
Operating System: Android

REPRODUCTION CASE
Unpack attached archive, copy properly signed chrome apk as base.apk and run crazyzip.py to create output.apk. (10MB attachment size limit doesn't allow me attaching generated apk)
Then install output.apk on Android device with ARM architecture, it will replace currently installed Chrome. Modified version will just run "id > /sdcard/chrome-id.txt" and exit. Note that this "update" will have access to files that normal Chrome has (such as cookies). Also note that installing this through on device installer will not work if there's already installed Chrome with same or greater versionCode; in that case modified APK can be installed through adb:
adb install -r output.apk
 
crazy-linker-zip.zip
3.3 KB Download

Comment 1 by palmer@chromium.org, Sep 29 2015

Cc: klo...@chromium.org digit@chromium.org tedc...@chromium.org
Labels: OS-Android Cr-Internals
Owner: simonb@chromium.org

Comment 3 by est...@chromium.org, Sep 29 2015

Labels: Security_Impact-Stable Security_Severity-High
Project Member

Comment 4 by ClusterFuzz, Sep 29 2015

Labels: Pri-1 M-45

Comment 5 by simonb@chromium.org, Sep 30 2015

Cc: andrewhayden@chromium.org
Important mitigation: The Play Store verifies the integrity of the entire APK file after downloading to secure storage and prior to installation, so it should not be possible to MitM the APK if it is downloaded from the Play Store. Therefore this attack would require either sideloading via ADB or installing Chrome from a third party app.

Comment 7 by simonb@chromium.org, Sep 30 2015

Status: Assigned
> In particular when looking for end-of-central-directory-record signature (PK\5\6), Chrome's Crazy Linker starts searching at file size minus length of signature and Android starts searching at file size minus size of EocdRecord.

Please can you explain why you see this difference as an indication that Android is immune to the APK change you have made but the crazy linker is vulnerable?

The EocdRecord is actually variable length due to the trailing 'comment' field. A comment can itself contain a full and valid EocdRecord. A search for EocdRecord must run backwards from the file end. The crazy linker begins searching at filelen-4, but while Android can start a bit back from this, if the 'valid' EocdRecord is amended so that its comment is another EocdRecord then both the crazy linker and Android will find this second EocdRecord and use it (because Android cannot start its search more than 20 bytes back from the file end, otherwise it would miss the EocdRecord it needs to find on unmodified APK files).

Thanks.

The vulnerability is that these zip parsers aren't working identically same, so we cannot say that one is 'immune' and other is 'vulnerable'. I've reported this here because Crazy Linker is embedded inside Chrome apk (through it's forked from Android, however zip support was added only on Chromium's fork), through maybe this should be reported to Android as well (to ban 'PK\4\5' inside zip comment).

The modified apk has at end an EocdRecord which is found by Android and in it's comment it has EocdRecord without last field (There's end of file where it should be).

So at the end of file we have:
struct EocdRecord { // Seen by Android
  uint32_t eocd_signature; // PK\5\6
  uint16_t disk_num;
  uint16_t cd_start_disk;
  uint16_t num_records_on_disk;
  uint16_t num_records;
  uint32_t cd_size;
  uint32_t cd_start_offset;
  // Android starts PK\5\6 scan here (filesize - sizeof(EocdRecord))
  uint16_t comment_length; // 20 (sizeof(EocdRecord) - 2)
};
struct EocdRecord { // Seen by Crazy Linker
  uint32_t eocd_signature; // PK\5\6
  uint16_t disk_num;
  uint16_t cd_start_disk;
  uint16_t num_records_on_disk;
  uint16_t num_records;
  uint32_t cd_size;
  // Crazy linker starts PK\5\6 scan here (filesize - sizeof("PK\4\5"))
  uint32_t cd_start_offset;
  // uint16_t comment_length; // Omitted, end of file instead
};
> The vulnerability is that these zip parsers aren't working identically same,...

Thank you for the extra information. It is now clearer as to how this divergent behaviour can affect things. When I first asked I was thinking of Android's unzip purely in the context of loading from APK (not a real unzip), rather than the more general SHA checking.

I am looking at ways to mitigate this in the chromium linker, but it seems that while I may be able to catch some things like that, ultimately if a file owned by an app is on the device then the app code has to trust it. While an APK's _contents_ can be trusted, the outer APK itself apparently has less protection.

As you note, banning zip comments from containing the EocdRecord signature would be advisable. Ideally we should prevent such files ever landing on the device at all, perhaps through either the package manager or the Play store (or maybe both). I will see what can be done in these areas.

In the meantime, I will investigate how hard it would be to make the crazy linker's zip functionality converge with Android's used for SHA checking.

Thanks again for the fuller explanation.

Project Member

Comment 10 by ClusterFuzz, Oct 2 2015

Labels: -M-45 M-46
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 6 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d9e316238aee59acf665d80b544cf4e1edfd3349

commit d9e316238aee59acf665d80b544cf4e1edfd3349
Author: Simon Baldwin <simonb@chromium.org>
Date: Tue Oct 06 10:23:26 2015

crazy linker: Alter search for zip EOCD start

When loading directly from APK, begin searching backwards
for the zip EOCD record signature at size of EOCD record
bytes before the end of the file.

BUG= 537205 
R=rmcilroy@chromium.org

Review URL: https://codereview.chromium.org/1390553002 .

Cr-Commit-Position: refs/heads/master@{#352577}

[modify] http://crrev.com/d9e316238aee59acf665d80b544cf4e1edfd3349/third_party/android_crazy_linker/README.chromium
[modify] http://crrev.com/d9e316238aee59acf665d80b544cf4e1edfd3349/third_party/android_crazy_linker/src/src/crazy_linker_zip.cpp

Status: Fixed
Project Member

Comment 13 by ClusterFuzz, Oct 6 2015

Labels: -Restrict-View-SecurityTeam Merge-Triage M-47 Restrict-View-SecurityNotify
Adding Merge-Triage label for tracking purposes.

Once your fix had sufficient bake time (on canary, dev as appropriate), please nominate your fix for merge by adding the Merge-Requested label.

When your merge is approved by the release manager, please start merging with higher milestone label first. Make sure to re-request merge for every milestone in the label list. You can get branch information on omahaproxy.appspot.com.

- Your friendly ClusterFuzz
Labels: Merge-Request-47

Comment 15 by tin...@google.com, Oct 7 2015

Labels: -Merge-Request-47 Merge-Approved-47 Hotlist-Merge-Approved
Approved for M47 (branch: 2526)
Project Member

Comment 16 by bugdroid1@chromium.org, Oct 7 2015

Labels: -Merge-Approved-47 merge-merged-2526
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7e39d690c4d9563954304ae31352ace3822caa07

commit 7e39d690c4d9563954304ae31352ace3822caa07
Author: Simon Baldwin <simonb@chromium.org>
Date: Wed Oct 07 17:00:34 2015

crazy linker: Alter search for zip EOCD start

When loading directly from APK, begin searching backwards
for the zip EOCD record signature at size of EOCD record
bytes before the end of the file.

BUG= 537205 
R=rmcilroy@chromium.org

Review URL: https://codereview.chromium.org/1390553002 .

Cr-Commit-Position: refs/heads/master@{#352577}
(cherry picked from commit d9e316238aee59acf665d80b544cf4e1edfd3349)

Review URL: https://codereview.chromium.org/1393813003 .

Cr-Commit-Position: refs/branch-heads/2526@{#28}
Cr-Branched-From: cb947c0153db0ec02a8abbcb3ca086d88bf6006f-refs/heads/master@{#352221}

[modify] http://crrev.com/7e39d690c4d9563954304ae31352ace3822caa07/third_party/android_crazy_linker/README.chromium
[modify] http://crrev.com/7e39d690c4d9563954304ae31352ace3822caa07/third_party/android_crazy_linker/src/src/crazy_linker_zip.cpp

Labels: Merge-Merged-47
Project Member

Comment 18 by bugdroid1@chromium.org, Oct 8 2015

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/bling/chromium.git/+/7e39d690c4d9563954304ae31352ace3822caa07

commit 7e39d690c4d9563954304ae31352ace3822caa07
Author: Simon Baldwin <simonb@chromium.org>
Date: Wed Oct 07 17:00:34 2015

Cc: kerz@chromium.org
Labels: -M-46 -Merge-Triage Release-0-M47
I understand that we're too late for M-46 for Android, so updating labels for an M-47 release. Adding kerz@ in case I'm incorrect.
Labels: reward-topanel
Adding reward-topanel for reward program consideration (details here: https://www.google.com/about/appsecurity/chrome-rewards/)

Comment 21 by palmer@google.com, Oct 15 2015

Cc: klyubin@google.com
Labels: -Security_Severity-High Security_Severity-Medium
Reducing severity based on mitigating factors and prerequisites for successful exploitation.
Labels: -reward-topanel reward-1000 reward-unpaid
Thanks for the report - our reward panel awarded you $1,000 for reporting this to us. Congratulations!

We'll credit you in our release notes as "Michal Bednarski". If you would prefer to use another name, please update this bug and I can update the release notes. We'll also provide a CVE ID in a few hours time for your reference.

Someone from our finance team should be in contact within a week to arrange payment. If this doesn't happen, please either update this bug or reach out to me directly at timwillis@

Thanks again for your report!

*** 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 established 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.
********************************* 
Labels: CVE-2015-6783
CVE-2015-6783
Labels: -reward-unpaid reward-inprocess
Labels: -reward-inprocess
Project Member

Comment 27 by ClusterFuzz, Jan 12 2016

Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Project Member

Comment 28 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 29 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
Labels: allpublic
Labels: CVE_description-submitted

Sign in to add a comment