CVE-2017-15924: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') (CWE-78)
value: In manager.c in ss-manager in shadowsocks-libev 3.1.0, improper parsing allows command injection via shell metacharacters in a JSON configuration request received via 127.0.0.1 UDP traffic, related to the add_server, build_config, and construct_command_line functions.
CWE description: The product constructs all or part of an OS command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended OS command when it is sent to a downstream component.
CVSS v3 base score: 7.8
Published: 2017-10-27T16:29Z
Repository: https://github.com/shadowsocks/shadowsocks-libev
Language: C
Files changed: src/manager.c
Commit date: 2017-10-14 07:21:08 +0800
Commit message: Fix #1734

Diff: diff --git a/src/manager.c b/src/manager.c
index f1d5dd1..5f9c9f0 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -90,11 +90,11 @@ destroy_server(struct server *server) {
     if (server->plugin_opts) ss_free(server->plugin_opts);
     if (server->mode) ss_free(server->mode);
 }
 
 static void
-build_config(char *prefix, struct server *server)
+build_config(char *prefix, struct manager_ctx *manager, struct server *server)
 {
     char *path    = NULL;
     int path_size = strlen(prefix) + strlen(server->port) + 20;
 
     path = ss_malloc(path_size);
@@ -108,35 +108,42 @@ build_config(char *prefix, struct server *server)
         return;
     }
     fprintf(f, "{\n");
     fprintf(f, "\"server_port\":%d,\n", atoi(server->port));
     fprintf(f, "\"password\":\"%s\"", server->password);
-    if (server->fast_open[0]) fprintf(f, ",\n\"fast_open\": %s", server->fast_open);
-    if (server->mode)   fprintf(f, ",\n\"mode\":\"%s\"", server->mode);
-    if (server->method) fprintf(f, ",\n\"method\":\"%s\"", server->method);
-    if (server->plugin) fprintf(f, ",\n\"plugin\":\"%s\"", server->plugin);
-    if (server->plugin_opts) fprintf(f, ",\n\"plugin_opts\":\"%s\"", server->plugin_opts);
+    if (server->method)
+        fprintf(f, ",\n\"method\":\"%s\"", server->method);
+    else if (manager->method)
+        fprintf(f, ",\n\"method\":\"%s\"", manager->method);
+    if (server->fast_open[0])
+        fprintf(f, ",\n\"fast_open\": %s", server->fast_open);
+    if (server->mode)
+        fprintf(f, ",\n\"mode\":\"%s\"", server->mode);
+    if (server->plugin)
+        fprintf(f, ",\n\"plugin\":\"%s\"", server->plugin);
+    if (server->plugin_opts)
+        fprintf(f, ",\n\"plugin_opts\":\"%s\"", server->plugin_opts);
     fprintf(f, "\n}\n");
     fclose(f);
     ss_free(path);
 }
 
 static char *
 construct_command_line(struct manager_ctx *manager, struct server *server)
 {
     static char cmd[BUF_SIZE];
-    char *method = manager->method;
     int i;
+    int port;
 
-    build_config(working_dir, server);
+    port = atoi(server->port);
+
+    build_config(working_dir, manager, server);
 
-    if (server->method) method = server->method;
     memset(cmd, 0, BUF_SIZE);
     snprintf(cmd, BUF_SIZE,
-             "%s -m %s --manager-address %s -f %s/.shadowsocks_%s.pid -c %s/.shadowsocks_%s.conf",
-             executable, method, manager->manager_address,
-             working_dir, server->port, working_dir, server->port);
+             "%s --manager-address %s -f %s/.shadowsocks_%d.pid -c %s/.shadowsocks_%d.conf",
+             executable, manager->manager_address, working_dir, port, working_dir, port);
 
     if (manager->acl != NULL) {
         int len = strlen(cmd);
         snprintf(cmd + len, BUF_SIZE - len, " --acl %s", manager->acl);
     }

Vulnerable code: build_config(char *prefix, struct server *server)
    if (server->fast_open[0]) fprintf(f, ",\n\"fast_open\": %s", server->fast_open);
    if (server->mode)   fprintf(f, ",\n\"mode\":\"%s\"", server->mode);
    if (server->method) fprintf(f, ",\n\"method\":\"%s\"", server->method);
    if (server->plugin) fprintf(f, ",\n\"plugin\":\"%s\"", server->plugin);
    if (server->plugin_opts) fprintf(f, ",\n\"plugin_opts\":\"%s\"", server->plugin_opts);
    char *method = manager->method;
    build_config(working_dir, server);
    if (server->method) method = server->method;
             "%s -m %s --manager-address %s -f %s/.shadowsocks_%s.pid -c %s/.shadowsocks_%s.conf",
             executable, method, manager->manager_address,
             working_dir, server->port, working_dir, server->port);
Fixed code: build_config(char *prefix, struct manager_ctx *manager, struct server *server)
    if (server->method)
        fprintf(f, ",\n\"method\":\"%s\"", server->method);
    else if (manager->method)
        fprintf(f, ",\n\"method\":\"%s\"", manager->method);
    if (server->fast_open[0])
        fprintf(f, ",\n\"fast_open\": %s", server->fast_open);
    if (server->mode)
        fprintf(f, ",\n\"mode\":\"%s\"", server->mode);
    if (server->plugin)
        fprintf(f, ",\n\"plugin\":\"%s\"", server->plugin);
    if (server->plugin_opts)
        fprintf(f, ",\n\"plugin_opts\":\"%s\"", server->plugin_opts);
    int port;
    port = atoi(server->port);

    build_config(working_dir, manager, server);
             "%s --manager-address %s -f %s/.shadowsocks_%d.pid -c %s/.shadowsocks_%d.conf",
             executable, manager->manager_address, working_dir, port, working_dir, port);