Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows Privilege Escalation #6361

Closed
iquaba opened this issue Jul 27, 2013 · 7 comments · Fixed by #6425
Closed

Windows Privilege Escalation #6361

iquaba opened this issue Jul 27, 2013 · 7 comments · Fixed by #6425
Assignees
Labels
Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Milestone

Comments

@iquaba
Copy link
Contributor

iquaba commented Jul 27, 2013

When using the cmd.script to upload and execute a script hosted on the salt-master, an attacker is able to overwrite the minion's copy of the script before execution. This leads to a race condition where the attacker may be able to run malicious code as system.

Sample script:

@thatch45
Copy link
Contributor

Yes, this is serious, we protect against this on unix like systems by making sure that the files are always owned by root and not otherwise writable and with umask considerations.

I will get @UtahDave on this ASAP and get this taken care of!

@ghost ghost assigned UtahDave Jul 27, 2013
@mike-ainsworth
Copy link
Contributor

In my experience, this issue is common for config management software running on Windows platforms. I have seen this situation with numerous other platforms, but I definitely agree that this is a security problem.

The challenge is that setting the permissions on a file in windows is usually a seperate action from copying a file and using subinacl or power-shell's set-acl to properly set the acl after the file is copied doesn't eliminate the race-condition vulnerability. Perhaps configuring the minion installation to set the destination directory that Salt is copying the script file to with admin and service account privileges and allowing inheritence do the rest would be the simplest solution.

@UtahDave
Copy link
Contributor

Mike, thanks for the great suggestion. That does sound like the best way
to go about this.

On Sat, Jul 27, 2013 at 3:54 PM, Mike Ainsworth notifications@github.comwrote:

In my experience, this issue is common for config management software
running on Windows platforms. I have seen this situation with numerous
other platforms, but I definitely agree that this is a security problem.

The challenge is that setting the permissions on a file in windows is
usually a seperate action from copying a file and using subinacl or
power-shell's set-acl to properly set the acl after the file is copied
doesn't eliminate the race-condition vulnerability. Perhaps configuring the
minion installation to set the destination directory that Salt is copying
the script file to with admin and service account privileges and allowing
inheritence do the rest would be the simplest solution.


Reply to this email directly or view it on GitHubhttps://github.com//issues/6361#issuecomment-21673207
.

Dave Boucha | Sr. Engineer

5272 South College Drive, Suite 301 | Murray, UT 84123
office 801-305-3563
dave@saltstack.com | www.saltstack.com http://saltstack.com/

@UtahDave
Copy link
Contributor

@mike-ainsworth and @iquaba, what are your thoughts on this fix? In the installer I set c:\salt to only be accessible by Administrator's Group and the System user.

@mike-ainsworth
Copy link
Contributor

That should do it!

-Mike

Sent from my iPhone. Please pardon any typographical errors.

On Jul 29, 2013, at 8:11 PM, David Boucha notifications@github.com wrote:

@mike-ainsworth https://github.com/mike-ainsworth and
@iquabahttps://github.com/iquaba,
what are your thoughts on this fix? In the installer I set c:\salt to only
be accessible by Administrator's Group and the System user.


Reply to this email directly or view it on
GitHubhttps://github.com//issues/6361#issuecomment-21763524
.

@iquaba
Copy link
Contributor Author

iquaba commented Jul 31, 2013

Sounds good to me. I'll take another whack at breaking it and make sure it holds up when I get a chance.

@UtahDave
Copy link
Contributor

Here's a link to an installer with this fix. http://saltstack.com/downloads/dev/Salt-Minion-0.16.2-rc1-AMD64-Setup.exe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants