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

Issue 46516 link

Starred by 17 users

Issue metadata

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

Blocked on:
issue 49107
issue 49346

issue 49176
issue 32413

  • Only users with EditIssue permission may comment.

Sign in to add a comment

Need to sync extension permissions

Project Member Reported by, Jun 14 2010

Issue description

For better security, we need to sync the set of permissions granted to an extension
Labels: OS-All
Cleaning up mstone:6 bugs, default assumption is that bugs w/ no os are os-all

Comment 2 by, Jul 14 2010

Labels: Security Restrict-View-Commit

Comment 3 by, Jul 14 2010

Extensions team agreed that this is a small enough risk to punt for mstone-6, but we may want to land any proto changes we need for this (will link to appropriate bug soon).

Comment 4 by, Jul 14 2010

Blockedon: 49107

Comment 5 by, Jul 14 2010

Labels: -Mstone-6 Mstone-7
Punting until mstone-7
@akalin, can you please help to explain the severity of this security issue. we need to make sure that it is safe from a security point of view to punt this to v7.

Comment 7 by, Jul 15 2010

So, consider the following sequence of events:

1) Client A installs extension E with version X on browser B1.
2) Client A sets up sync on browser B2 and starts the process to download extension E.
3) The auto-update server hosting E bumps the version to X+1.
4) Browser B2 downloads extension E, but gets version X+1.  It installs the extension silently, even of version X+1 requires more permissions (e.g., accesses history, requires a plugin, etc.)

The real fix is to also sync a set of permissions and use that to determine whether to auto-disable a synced extension and add an infobar (as is now done when a regular auto-update happens and an updated version needs more permissions).

The mitigation is that the window for this to happen is pretty small, and is possible only on an initial sync of an extension.

Let me know if this wasn't clear, or if you need more details!
Thanks @akalin for the detailed explanation. Another question, In the current scenario, when an extension gets autoupdated and lets say newer permissions gets added to the extension manifest by the extension author, then what do we do [for the single-computer case]. Do we show an infobar if user is comfortable with granting the extension these newer permissions or ... ? 

Comment 9 by, Jul 15 2010

The newer permissions are compared with the existing ones, and if it's significant enough (based on some logic) then the extension is disabled and an infobar is shown explaining the situation.  The user can re-enable the extension if he feels it's okay.
Labels: -Restrict-View-Commit Restrict-View-SecurityTeam SecSeverity-Low
We looked at it and it does not look exploitable on its own. It could be leveraged by combining it with other vulnerability. So, we are fine with pushing it to M7 and marking secseverity-low.
Labels: -Pri-2 -Mstone-7 Pri-1 Mstone-6
Talked with the extension folks again, and we failed to take into account third-party auto-update servers.  If an attacker controls the auto-update server that a particular extension points to, he could detect new installs and install a malicious upgrade.

Short-term solution for m6 is to not sync on version mismatch for non-google update servers.  That may inconvenience a small number of honest devs (as it will cause sync delays for their extension) but it's better than the alternative.
Blockedon: 49346
Fix for 49346 may also cover this case.
Labels: WillMerge
Labels: -WillMerge ForMerge
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

It's a really bad idea to juggle bugs between open and closed states, because it makes tracking really confusing. When this bug is fixed we'll just close it, and file a new bug for the follow-up changes.
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 27 by, Oct 13 2012

Blockedon: -chromium:49107 -chromium:49346 chromium:49107 chromium:49346
Blocking: -chromium:32413 -chromium:49176 chromium:32413 chromium:49176
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 28 by, Mar 10 2013

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

Comment 29 by, Mar 13 2013

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

Comment 30 by, Mar 21 2013

Labels: -Security-Severity-Low Security_Severity-Low
Project Member

Comment 31 by, Mar 21 2013

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

Comment 32 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 33 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