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

Issue 49346 link

Starred by 1 user

Issue metadata

Status: Fixed
Last visit > 30 days ago
Closed: Jul 2010
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security

issue 46516
issue 49176

  • Only users with EditIssue permission may comment.

Sign in to add a comment

Sync allows an attacker who compromises Google credentials to push extensions to a user's browser

Reported by, Jul 16 2010

Issue description

Essentially the ability to sync extensions means that knowledge of Google credentials is equivalent to the ability to install arbitrary extensions into any browser the user touches. I realize that in theory user's should protect their credentials and treat them with respect but in practice password reuse is rampant.

The attack scenario is essentially

1. attacker compromises users credentials (reuse on a forum, logging in over public wifi, etc).
2. attacker syncs a chrome instance and installs an evil extension with arbitrary permissions.
3. user fires up chrome and syncs in one or more places. The extension can now access anything the user does (sniff additional credentials or CC info, access autofill data, etc).

I think we need to have a discussion on how we can limit the risk here. By doing something like only allowing extensions that are in the extension repository to be automatically synced (the rest could be disabled by default until the user manually enables them) we would at least have the ability to blacklist extensions once we know they are bad.


Comment 1 by, Jul 16 2010

Labels: Feature-Extensions

Comment 2 by, Jul 16 2010

Comment 3 by, Jul 16 2010

Comment 4 by, Jul 16 2010


This seems a little related to password syncing. Do we do anything extra there? Because if not, getting Google credentials leads to all kinds of other interesting access.

Comment 5 by, Jul 16 2010

Oh, and fwiw, we can already block any extensions, not just those from the gallery. Every extension has a unique ID. This doesn't stop a developer from just creating multiple extensions, but the gallery wouldn't help that much with that either.

Comment 6 by, Jul 16 2010

I think it isn't too bad a user-experience to just sync them initially disabled. We could use a desktop notification to tell users that new extensions have been synced, and have a link to review/enable them.

That does imply, though, that the enabled state should _not_ be synced. It is per-machine and starts off disabled.
One approach would be to require the install dialog for new extensions being synced.  If we're worried about that being too intrusive, we could do it only for high-permission extensions, but my guess is that the bar would wind up being low enough that we may as well prompt all of them.

Agree with aa about passwords being essentially as bad (minus NPAPI).
Another idea.  Given that NPAPI is the only thing really significantly worse, we could just disable syncing NPAPI extensions.  That would be a really easy change to make with no new UI, and only confusing to the relatively small number of users who use NPAPI extensions. (one popular exception is cooliris)

Comment 9 by, Jul 16 2010

To summarize the discussion for the security folks, the proposed M6 fix is:

- disable an extension newly-installed via sync (and pop up an infobar) if (extension uses NPAPI) or (extension is not from the gallery)

The proposed long-term (i.e., M7) fix is:

- disable any extension newly-installed via sync and prompt the user when this happens (UI TBD)

What do you guys think?
Actually the M7 fix should be "hold for approval any extension..." instead of "disable any extension..." since an approved extension may still end up being disabled (if it was disabled on the original install location).
Another correction: M6 fix will involve no UI; the extension will just be disabled.
Has a chat with Justin, Cris. We agree with your suggestion.

So, for sync in M6.
1. NPAPI extensions - disabled.
2. Extensions hosted on third-party servers - disabled. (Also covers  bug 46516 ).

Actually, a simpler fix for M6 would be to not sync NPAPI extensions/third-party extensions at all (as opposed to syncing them and disabling).  That should be fine, right security folks?  I'll go ahead and do that.
from a security perspective, either is fine as long as it works for the user experience.
Switching back owner to akalin since he has the same fix ready and has LGTM too :)
Labels: WillMerge
Labels: -WillMerge ForMerge
Status: WillMerge
Labels: -ForMerge
Labels: ForMerge
Status: Available
Sorry, changed the status on the wrong bug.
Status: WillMerge
For now, lets keep status to willmerge so as to help us to track merge to already cut m6 branch. After it is merged, we should reopen this bug to track the M7 counterpart [and accordingly change the milestone tag].

Also, looks like bugdroid is down, so updating the commit info below.

Disallow syncing of extensions with third-party update URLs or plugins.

BUG= 49346 ,  46516 
TEST=new unit tests, manual


Added checks to handle unsyncable extensions.

BUG= 49346 ,  46516 
TEST=manual, unit tests

Status: Fixed
Marking this fixed since 472 changes merged in and pending m7 items tracked at

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: Type-Security
Labels: SecImpacts-None
Batch update.
Labels: -Restrict-View-SecurityNotify
Lifting view restrictions.
Project Member

Comment 33 by, Oct 13 2012

Blocking: -chromium:49176 -chromium:46516 chromium:49176 chromium:46516
Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Project Member

Comment 34 by, Mar 10 2013

Labels: -Area-Internals -Feature-Sync -SecSeverity-Medium -Mstone-6 -Feature-Extensions -Type-Security -SecImpacts-None Cr-Platform-Extensions M-6 Cr-Services-Sync Security-Severity-Medium Cr-Internals Security-Impact-None Type-Bug-Security
Project Member

Comment 35 by, Mar 13 2013

Labels: -Restrict-AddIssueComment-Commit Restrict-AddIssueComment-EditIssue
Project Member

Comment 36 by, Mar 21 2013

Labels: Security_Severity-None
Project Member

Comment 37 by, Mar 21 2013

Labels: -Security-Impact-None Security_Impact-None
Project Member

Comment 38 by, Mar 21 2013

Labels: -Security-Severity-Medium -Security_Severity-None Security_Severity-Medium
Project Member

Comment 39 by, Oct 1 2016

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit - Your friendly Sheriffbot
Project Member

Comment 40 by, Oct 2 2016

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit - Your friendly Sheriffbot
Labels: allpublic

Sign in to add a comment