Fix OS command injection in fix_filename() - use allowlist for safe chars

Agent-Logs-Url: https://github.com/tbamud/tbamud/sessions/a9e10199-b353-440a-ba26-279f0d0e42bf

Co-authored-by: welcor <357770+welcor@users.noreply.github.com>

.github/workflows/build.yml

@@ -3,7 +3,7 @@ name: C/C++ CI
 on:
   push:
     branches: [ "master" ]
-  pull_request:
+  pull_request_target:
     branches: [ "master" ]
 
 permissions:

src/dg_mobcmd.c

@@ -1024,17 +1024,27 @@ ACMD(do_mdoor)
     }
 
     if ((rm = get_room(target)) == NULL) {
-        mob_log(ch, "mdoor: invalid target");
+        mob_log(ch, "mdoor: invalid target (arg == %s)", target);
         return;
     }
 
     if ((dir = search_block(direction, dirs, FALSE)) == -1) {
-        mob_log(ch, "mdoor: invalid direction");
+        char dirs_str[256];
+        int di, doff = 0;
+        dirs_str[0] = '\0';
+        for (di = 0; *dirs[di] != '\n'; di++)
+            doff += snprintf(dirs_str + doff, sizeof(dirs_str) - doff, "%s%s", doff ? " " : "", dirs[di]);
+        mob_log(ch, "mdoor: invalid direction (arg == %s) not found in: [ %s ]", direction, dirs_str);
         return;
     }
 
     if ((fd = search_block(field, door_field, FALSE)) == -1) {
-        mob_log(ch, "odoor: invalid field");
+        char fields_str[256];
+        int fi, foff = 0;
+        fields_str[0] = '\0';
+        for (fi = 0; *door_field[fi] != '\n'; fi++)
+            foff += snprintf(fields_str + foff, sizeof(fields_str) - foff, "%s%s", foff ? " " : "", door_field[fi]);
+        mob_log(ch, "mdoor: invalid field (arg == %s) not found in: [ %s ]", field, fields_str);
         return;
     }
 
@@ -1081,8 +1091,10 @@ ACMD(do_mdoor)
         case 5:  /* room        */
             if ((to_room = real_room(atoi(value))) != NOWHERE)
                 newexit->to_room = to_room;
-            else
-                mob_log(ch, "mdoor: invalid door target");
+            else {
+                newexit->to_room = NOWHERE;
+                mob_log(ch, "mdoor: invalid door target (arg == %s)", value);
+            }
             break;
         }
     }

src/dg_objcmd.c

@@ -625,17 +625,27 @@ static OCMD(do_odoor)
     }
 
     if ((rm = get_room(target)) == NULL) {
-        obj_log(obj, "odoor: invalid target");
+        obj_log(obj, "odoor: invalid target (arg == %s)", target);
         return;
     }
 
     if ((dir = search_block(direction, dirs, FALSE)) == -1) {
-        obj_log(obj, "odoor: invalid direction");
+        char dirs_str[256];
+        int di, doff = 0;
+        dirs_str[0] = '\0';
+        for (di = 0; *dirs[di] != '\n'; di++)
+            doff += snprintf(dirs_str + doff, sizeof(dirs_str) - doff, "%s%s", doff ? " " : "", dirs[di]);
+        obj_log(obj, "odoor: invalid direction (arg == %s) not found in: [ %s ]", direction, dirs_str);
         return;
     }
 
     if ((fd = search_block(field, door_field, FALSE)) == -1) {
-        obj_log(obj, "odoor: invalid field");
+        char fields_str[256];
+        int fi, foff = 0;
+        fields_str[0] = '\0';
+        for (fi = 0; *door_field[fi] != '\n'; fi++)
+            foff += snprintf(fields_str + foff, sizeof(fields_str) - foff, "%s%s", foff ? " " : "", door_field[fi]);
+        obj_log(obj, "odoor: invalid field (arg == %s) not found in: [ %s ]", field, fields_str);
         return;
     }
 
@@ -682,8 +692,10 @@ static OCMD(do_odoor)
         case 5:  /* room        */
             if ((to_room = real_room(atoi(value))) != NOWHERE)
                 newexit->to_room = to_room;
-            else
-                obj_log(obj, "odoor: invalid door target");
+            else {
+                newexit->to_room = NOWHERE;
+                obj_log(obj, "odoor: invalid door target (arg == %s)", value);
+            }
             break;
         }
     }

src/dg_wldcmd.c

@@ -224,17 +224,27 @@ WCMD(do_wdoor)
     }
 
     if ((rm = get_room(target)) == NULL) {
-        wld_log(room, "wdoor: invalid target");
+        wld_log(room, "wdoor: invalid target (arg == %s)", target);
         return;
     }
 
     if ((dir = search_block(direction, dirs, FALSE)) == -1) {
-        wld_log(room, "wdoor: invalid direction");
+        char dirs_str[256];
+        int di, doff = 0;
+        dirs_str[0] = '\0';
+        for (di = 0; *dirs[di] != '\n'; di++)
+            doff += snprintf(dirs_str + doff, sizeof(dirs_str) - doff, "%s%s", doff ? " " : "", dirs[di]);
+        wld_log(room, "wdoor: invalid direction (arg == %s) not found in: [ %s ]", direction, dirs_str);
         return;
     }
 
     if ((fd = search_block(field, door_field, FALSE)) == -1) {
-        wld_log(room, "wdoor: invalid field");
+        char fields_str[256];
+        int fi, foff = 0;
+        fields_str[0] = '\0';
+        for (fi = 0; *door_field[fi] != '\n'; fi++)
+            foff += snprintf(fields_str + foff, sizeof(fields_str) - foff, "%s%s", foff ? " " : "", door_field[fi]);
+        wld_log(room, "wdoor: invalid field (arg == %s) not found in: [ %s ]", field, fields_str);
         return;
     }
 
@@ -281,8 +291,10 @@ WCMD(do_wdoor)
         case 5:  /* room        */
             if ((to_room = real_room(atoi(value))) != NOWHERE)
                 newexit->to_room = to_room;
-            else
-                wld_log(room, "wdoor: invalid door target");
+            else {
+                newexit->to_room = NOWHERE;
+                wld_log(room, "wdoor: invalid door target (arg == %s)", value);
+            }
             break;
         }
     }

src/genolc.c

@@ -277,7 +277,10 @@ int sprintascii(char *out, bitvector_t bits)
   return j;
 }
 
-/* converts illegal filename chars into appropriate equivalents */ 
+/* converts illegal filename chars into appropriate equivalents.
+ * Uses an allowlist: alphanumerics, underscore, hyphen, and dot are kept;
+ * spaces are converted to underscores; all other characters (including shell
+ * metacharacters such as ; | & ` $ > < \n) are silently dropped. */ 
 static void fix_filename(const char *str, char *outbuf, size_t maxlen)
 { 
   const char *in = str;
@@ -285,21 +288,17 @@ static void fix_filename(const char *str, char *outbuf, size_t maxlen)
   int count = 0;
 
   while (*in) {
-    switch(*in) {
-      case ' ': *out = '_'; out++; break;
-      case '(': *out = '{'; out++; break;
-      case ')': *out = '}'; out++; break;
- 
-      /* skip the following */ 
-      case '\'':             break; 
-      case '"':              break; 
- 
-      /* Legal character */ 
-      default: *out = *in;  out++;break;
-    } 
+    if (isalnum((unsigned char)*in) || *in == '_' || *in == '-' || *in == '.') {
+      /* Safe characters kept as-is */
+      *out++ = *in;
+      if (++count == maxlen - 1) break;
+    } else if (*in == ' ') {
+      /* Spaces become underscores */
+      *out++ = '_';
+      if (++count == maxlen - 1) break;
+    }
+    /* All other characters, including shell metacharacters, are dropped */
     in++;
-    count++;
-    if (count == maxlen - 1) break;
   }
   *out = '\0';
 }