More example of argument and shell command injections in browser invocation

Published: May 30 2018

Updated: May 30 2018

In the previous episode, I talked about some argument and shell command injections vulnerabilities through URIs passed to browsers. Here I am evaluating some other CVEs which were registered at the same time (not by me).

ScummVM (CVE-2017-17528)

In ScummVM, we have:

bool OSystem_POSIX::openUrl(const Common::String &url) {
	// inspired by Qt's "qdesktopservices_x11.cpp"

	// try "standards"
	if (launchBrowser("xdg-open", url))
		return true;
	if (launchBrowser(getenv("DEFAULT_BROWSER"), url))
		return true;
	if (launchBrowser(getenv("BROWSER"), url))
		return true;

	// try desktop environment specific tools
	if (launchBrowser("gnome-open", url)) // gnome
		return true;
	if (launchBrowser("kfmclient openURL", url)) // kde
		return true;
	if (launchBrowser("exo-open", url)) // xfce
		return true;

	// try browser names
	if (launchBrowser("firefox", url))
		return true;
	if (launchBrowser("mozilla", url))
		return true;
	if (launchBrowser("netscape", url))
		return true;
	if (launchBrowser("opera", url))
		return true;
	if (launchBrowser("chromium-browser", url))
		return true;
	if (launchBrowser("google-chrome", url))
		return true;

	warning("openUrl() (POSIX) failed to open URL");
	return false;
}

bool OSystem_POSIX::launchBrowser(const Common::String& client, const Common::String &url) {
	// FIXME: system's input must be heavily escaped
	// well, when url's specified by user
	// it's OK now (urls are hardcoded somewhere in GUI)
	Common::String cmd = client + " " + url;
	return (system(cmd.c_str()) != -1);
}

OSystem_POSIX::openUrl() calls system() without quoting the URI. This is vulnerable to shell command injection but, as stated in the comment, it is currently not a problem in practice because the only calls of openUrl() are:

g_system->openUrl("http://www.amazon.de/EuroVideo-Bildprogramm-GmbH-Full-Pipe/dp/B003TO51YE/ref=sr_1_1?ie=UTF8&s=videogames&qid=1279207213&sr=8-1");
g_system->openUrl("http://pipestudio.ru/fullpipe/");
g_system->openUrl("http://scummvm.org/")
g_system->openUrl(getUrl())

with:

Common::String StorageWizardDialog::getUrl() const {
	Common::String url = "https://www.scummvm.org/c/";
	switch (_storageId) {
	case Cloud::kStorageDropboxId:
		url += "db";
		break;
	case Cloud::kStorageOneDriveId:
		url += "od";
		break;
	case Cloud::kStorageGoogleDriveId:
		url += "gd";
		break;
	case Cloud::kStorageBoxId:
		url += "bx";
		break;
	}

	if (Cloud::CloudManager::couldUseLocalServer())
		url += "s";

	return url;
}

The only case where shell commands are actually injected is the first one where it does something like:

xdg-open https://www.amazon.de/EuroVideo-Bildprogramm-GmbH-Full-Pipe/dp/B003TO51YE/ref=sr_1_1?ie=UTF8&s=videogames&qid=1279207213&sr=8-1

which make these assignments in subshells (which is quite harmless):

ie=UTF8
s=videogames
qid=1279207213
sr=8-1

References:

GNU GLOBAL (CVE-2017-17531)

In GNU GLOBAL, it looked like this:

snprintf(com, sizeof(com), "%s \"%s\"", browser, url);
system(com);

Here, the URI is double-quoted but this is not enough:

it is possible to escape out of the quoted string by using double quotes in the URI;
it is possible to inject commands with command substitutions, $(...).

For v6.6.1, each argument is quoted with quote_shell() in order to properly escape the shell metacharacters:

strbuf_puts(sb, quote_shell(browser));
strbuf_putc(sb, ' ');
strbuf_puts(sb, quote_shell(url));
system(strbuf_value(sb));

In v6.6.2 this was changed to using execvp():

argv[0] = (char *)browser;
argv[1] = (char *)url;
argv[2] = NULL;
execvp(browser, argv);

Using execvp() is much better than relying on system() and using an error-prone escaping of the URI to prevent injections.

References:

gjots2 (CVE-2017-17535)

In gjots2, the vulnerable code is:

def _run_browser_on(self, url):
  if self.debug:
    print inspect.getframeinfo(inspect.currentframe())[2]
  browser = self._get_browser()
  if browser:
    os.system(browser + " '" + url + "' &")
  else:
    self.msg("Can't run a browser")
  return 0

The URI is single-quoted.

We can use single-quotes in the URI to injection commands. For example, opening link in gjots2 spawns a xterm:

http://www.example.com/'&xterm'

References:

ABiWord (CVE-2017-17529)

In AbiWord, we have:

GError *err = NULL;
#if GTK_CHECK_VERSION(2,14,0)
if(!gtk_show_uri (NULL, url, GDK_CURRENT_TIME, &err)) {
  fallback_open_uri(url, &err);
}
return err;
#elif defined(WITH_GNOMEVFS)
gnome_vfs_url_show (url);
return err;
#else
fallback_open_uri(url, &err);
return err;
#endif

The problematic code is supposed to be in fallback_open_uri():

gint    argc;
gchar **argv = NULL;
char   *cmd_line = g_strconcat (browser, " %1", NULL);

if (g_shell_parse_argv (cmd_line, &argc, &argv, err)) {
  /* check for '%1' in an argument and substitute the url
   * otherwise append it */
  gint i;
  char *tmp;

  for (i = 1 ; i < argc ; i++)
    if (NULL != (tmp = strstr (argv[i], "%1"))) {
      *tmp = '\0';
      tmp = g_strconcat (argv[i],
        (clean_url != NULL) ? (char const *)clean_url : url,
        tmp+2, NULL);
      g_free (argv[i]);
      argv[i] = tmp;
      break;
    }

  /* there was actually a %1, drop the one we added */
  if (i != argc-1) {
    g_free (argv[argc-1]);
    argv[argc-1] = NULL;
  }
  g_spawn_async (NULL, argv, NULL, G_SPAWN_SEARCH_PATH,
    NULL, NULL, NULL, err);
  g_strfreev (argv);
}
g_free (cmd_line);

This code seems correct with respect to injection through the URI: the URI string cannot be expanded into multiple arguments (no word splitting) and is not passed to system().

I think this code is safe. I could not trigger any injection through AbiWord. I tested gtk_show_uri(), fallback_open_uri() and gnome_vfs_url_show() in isolation and I could not trigger any injection through the URI.

References:

FontForge (CVE-2017-17521)

In FontForge, the help() function is vulnerable. The URI is double-quoted:

temp = malloc(strlen(browser) + strlen(fullspec) + 20);
sprintf( temp, strcmp(browser,"kfmclient openURL")==0 ? "%s \"%s\" &" : "\"%s\" \"%s\" &", browser, fullspec );
system(temp);

In practice, it is always used with path where this is safe to do.

References:

CVE-2017-17521

Ocaml Batteries Included (CVE-2017-17519)

The code is:

let (browser: (_, _, _) format) = "@BROWSER_COMMAND@ %s";;

(**The default function to open a www browser.*)
let default_browse s =
  let command = Printf.sprintf browser s in
  Sys.command command
let current_browse = ref default_browse

let browse s = !current_browse s

system() is called without any quotation of the URI.

Example:

open Batteries;;
open BatteriesConfig;;
browse "http://www.example.com/&xterm";;

Compiled with:

ocamlfind ocamlc -package batteries -linkpkg browser2.ml -o browser2

References:

CVE-2017-17519

Python 3 (CVE-2017-17522)

The code is:

class GenericBrowser(BaseBrowser):
    """Class for all browsers started with a command
       and without remote functionality."""

    def __init__(self, name):
        if isinstance(name, str):
            self.name = name
            self.args = ["%s"]
        else:
            # name should be a list with arguments
            self.name = name[0]
            self.args = name[1:]
        self.basename = os.path.basename(self.name)

    def open(self, url, new=0, autoraise=True):
        cmdline = [self.name] + [arg.replace("%s", url)
                                 for arg in self.args]
        try:
            if sys.platform[:3] == 'win':
                p = subprocess.Popen(cmdline)
            else:
                p = subprocess.Popen(cmdline, close_fds=True)
            return not p.wait()
        except OSError:
            return False

A note in the CVE says:

NOTE: a software maintainer indicates that exploitation is impossible because the code relies on subprocess.Popen and the default shell=False setting.

Popen is indeed passed an array of arguments which are passed to execve(). There is not argument splitting and no shell is involved so this code is not vulnerable to URI-based injections.

References:

CVE-2017-17522

TeX (CVE-2017-17513)

I have no idea what mtxrun is supposed to do but it looks like it is vulnerable because the URI is not quoted:

local launchers={
  windows="start %s",
  macosx="open %s",
  unix="$BROWSER %s &> /dev/null &",
}
function os.launch(str)
  execute(format(launchers[os.name] or launchers.unix,str))
end

References:

CVE-2017-17513

Summary

ScummVM (CVE-2017-17528), shell command injection because of unquoted system() but not vulnerable in practice
GNU GLOBAL (CVE-2017-17531), shell command injection because of system() call with double-quoted URI
gjots2 (CVE-2017-17535): shell command injection because of system() call with single-quoted URI
FontForge (CVE-2017-17521): shell command injection because of system() call with double-quoted URI but not vulnerable in practice
Ocaml Batteries Included (CVE-2017-17519): shell command injection because of unquoted system() call
Python 3 (CVE-2017-17522): not vulnerable (relies on execve())
TeX (CVE-2017-17513): shell command injection because of unquoted system() call