sambactrl: Fix local priviledge escalation

From the reporter:
   LPE in /usr/local/bin/sambactrl 'join' action
   File: src/misc-progs/sambactrl.c, lines 117-126.
   All other actions call is_valid_argument_alnum() on argv[2]. The
   'join' branch skips it entirely and feeds argv[2]/argv[3] into
   snprintf + safe_system (which is /bin/sh -c). Binary is installed
   -m 4750 -g nobody (src/misc-progs/Makefile:41), so any nobody-context
   process can invoke it and escalate to root.

Reported-by: valent1 <gooads612@gmail.com>
Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>

diff --git a/src/misc-progs/sambactrl.c b/src/misc-progs/sambactrl.c
index 38c26089cdb9e7717e49da71f2c69a94b6fa959c..11b0b4e010b7926e2dc38204acd1f6f284a2b47f 100644 (file)
--- a/src/misc-progs/sambactrl.c
+++ b/src/misc-progs/sambactrl.c
@@ -11,6 +11,9 @@
 char command[BUFFER_SIZE];
 
 int main(int argc, char *argv[]) {
+       char who[BUFFER_SIZE];
+       int r;
+
        if (!(initsetuid()))
                exit(1);
 
@@ -116,9 +119,21 @@ int main(int argc, char *argv[]) {
 
        } else if (strcmp(argv[1], "join") == 0) {
                if (argc == 4) {
-                       snprintf(command, BUFFER_SIZE - 1, "/usr/bin/net join -U \"%s%%%s\"",
-                               argv[2], argv[3]);
-                       return safe_system(command);
+                       // Format who is joining
+                       r = snprintf(who, sizeof(who), "%s%%%s", argv[2], argv[3]);
+                       if (r < 0)
+                               return r;
+
+                       // Compose command line
+                       char* args[] = {
+                               "join",
+                               "-U",
+                               who,
+                               NULL,
+                       };
+
+                       // Run the operation
+                       return run("/usr/bin/net", args);
                } else {
                        fprintf(stderr, "Wrong number of arguments. Need username and password.\n");
                        return 1;