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 1 user
Status: Fixed
Owner:
Closed: Sep 2014
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Bug-Security

Blocked on:
issue 141168



Sign in to add a comment
Security: Unquoted path in mini_installer can lead to executing the wrong executable
Reported by cdn@chromium.org, Feb 3 2014 Back to list
In mini_installer.cc UnpackBinaryResources() a path is executed through CreateProcess with a null first argument. THis affectively turns create process into a ShellExecute of the cmd line argument. 

The binary file to be run in the cmdline is not quoted which means the path can be incorrectly resolved. For example C:\Program Files\blah.exe would first be resolved as c:\Program.exe if this file exists.

This is of lw risk given that in the default case (chrome installed on the system drive) the root of the drive where program.exe would have to be placed already requires admin privileges to be written.

In some configurations however this could allow a privilege escalation to Local System. 


 
Comment 1 by cdn@chromium.org, Feb 4 2014
Status: Started
Comment 2 by cdn@chromium.org, Feb 5 2014
Cc: tommi@chromium.org grt@chromium.org
Project Member Comment 3 by bugdroid1@chromium.org, Feb 11 2014
------------------------------------------------------------------------
r250500 | cdn@chromium.org | 2014-02-11T21:03:13.983041Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/mini_installer/mini_installer.cc?r1=250500&r2=250499&pathrev=250500

Add quotes around executable pathes in the mini installer

BUG= 340387 
TEST=N/A

Review URL: https://codereview.chromium.org/154113004
------------------------------------------------------------------------
Project Member Comment 4 by bugdroid1@chromium.org, Feb 14 2014
------------------------------------------------------------------------
r251369 | grt@chromium.org | 2014-02-14T18:14:03.246323Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/mini_installer/mini_installer.cc?r1=251369&r2=251368&pathrev=251369

Revert 250500 "Add quotes around executable pathes in the mini i..."

Speculative revert to see if this resolves a spike in update failures on
canary.

> Add quotes around executable pathes in the mini installer
> 
> BUG= 340387 
> TEST=N/A
> 
> Review URL: https://codereview.chromium.org/154113004

TBR=cdn@chromium.org

Review URL: https://codereview.chromium.org/167333002
------------------------------------------------------------------------
Project Member Comment 5 by bugdroid1@chromium.org, Feb 14 2014
Labels: merge-merged-1840
------------------------------------------------------------------------
r251371 | dxie@chromium.org | 2014-02-14T18:28:10.065829Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1840/src/chrome/installer/mini_installer/mini_installer.cc?r1=251371&r2=251370&pathrev=251371

Merge 251369 "Revert 250500 "Add quotes around executable pathes..."

> Revert 250500 "Add quotes around executable pathes in the mini i..."
> 
> Speculative revert to see if this resolves a spike in update failures on
> canary.
> 
> > Add quotes around executable pathes in the mini installer
> > 
> > BUG= 340387 
> > TEST=N/A
> > 
> > Review URL: https://codereview.chromium.org/154113004
> 
> TBR=cdn@chromium.org
> 
> Review URL: https://codereview.chromium.org/167333002

TBR=grt@chromium.org

Review URL: https://codereview.chromium.org/167353002
------------------------------------------------------------------------
Comment 6 by grt@chromium.org, Feb 17 2014
Blockedon: chromium:141168
Project Member Comment 7 by ClusterFuzz, May 13 2014
Labels: -M-34 M-35
Project Member Comment 8 by ClusterFuzz, Jun 23 2014
Labels: -M-35 M-36
grt@: Any idea what the status of this bug is?
Project Member Comment 10 by ClusterFuzz, Jul 28 2014
Labels: -Security_Impact-Beta
Project Member Comment 11 by ClusterFuzz, Aug 18 2014
Labels: -M-36 M-37
Cc: -grt@chromium.org
Owner: grt@chromium.org
Status: Assigned
grt: Would you be a good owner for this? If not, could you please help with triage?
Comment 13 by grt@chromium.org, Sep 24 2014
Cc: -tommi@chromium.org karen@chromium.org
I believe http://crrev.com/40d8624d unblocked this fix. I'll try re-landing cdn's change. We'll have to monitor canary diff update failures to see if they rise again.

Karen: please revert the change I'm about to land if you see a diff update failure spike. Thanks.
Project Member Comment 14 by bugdroid1@chromium.org, Sep 24 2014
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8b12f969b04be8c9a8a45931665844c9515f926b

commit 8b12f969b04be8c9a8a45931665844c9515f926b
Author: grt <grt@chromium.org>
Date: Wed Sep 24 15:59:40 2014

Specify the path to the old setup.exe in the lpApplicationName parameter to CreateProcess.

This is a reland of http://crrev.com/3882aae7 since
http://crrev.com/40d8624d should have fixed the problem leading to
diff update failures.

BUG= 340387 
TEST=N/A

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

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

[modify] https://chromium.googlesource.com/chromium/src.git/+/8b12f969b04be8c9a8a45931665844c9515f926b/chrome/installer/mini_installer/mini_installer.cc

Project Member Comment 15 by ClusterFuzz, Sep 29 2014
Labels: -M-37 M-38
Comment 16 by grt@chromium.org, Sep 29 2014
Status: Fixed
Canary updates look good. Calling this fixed.
Project Member Comment 17 by ClusterFuzz, Sep 29 2014
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -M-38 M-39
Labels: Release-0-M39
Project Member Comment 20 by ClusterFuzz, Jan 5 2015
Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Project Member Comment 21 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 22 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
Sign in to add a comment