Fix stray null write from sizeof(pointer) bug in var_subst() (#190)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: welcor <357770+welcor@users.noreply.github.com>

src/dg_variables.c

@@ -1722,5 +1722,5 @@ void var_subst(void *go, struct script_data *sc, trig_data *trig,
       left -= len;
     } /* else if *p .. */
   } /* while *p .. */
-  buf[sizeof(buf) - 1] = '\0';
+  *buf = '\0';
 }

src/interpreter.c

@@ -41,7 +41,7 @@
 /* local (file scope) functions */
 static int perform_dupe_check(struct descriptor_data *d);
 static struct alias_data *find_alias(struct alias_data *alias_list, char *str);
-static void perform_complex_alias(struct txt_q *input_q, char *orig, struct alias_data *a, struct char_data *ch);
+static int perform_complex_alias(struct txt_q *input_q, char *orig, struct alias_data *a);
 static int _parse_name(char *arg, char *name);
 static bool perform_new_char_dupe_check(struct descriptor_data *d);
 /* sort_commands utility */
@@ -668,7 +668,7 @@ ACMD(do_alias)
  * commands. */
 #define NUM_TOKENS       9
 
-static void perform_complex_alias(struct txt_q *input_q, char *orig, struct alias_data *a, struct char_data *ch)
+static int perform_complex_alias(struct txt_q *input_q, char *orig, struct alias_data *a)
 {
   struct txt_q temp_queue;
   struct txt_block *qtmp;
@@ -732,16 +732,16 @@ static void perform_complex_alias(struct txt_q *input_q, char *orig, struct alia
     temp_queue.tail->next = input_q->head;
     input_q->head = temp_queue.head;
   }
-  return;
+  return (0);
 
 overflow:
-  send_to_char(ch, "Alias expansion too long.\r\n");
   while (temp_queue.head) {
     qtmp = temp_queue.head;
     temp_queue.head = qtmp->next;
     free(qtmp->text);
     free(qtmp);
   }
+  return (-1);
 }
 
 /* Given a character and a string, perform alias replacement on it.
@@ -777,7 +777,8 @@ int perform_alias(struct descriptor_data *d, char *orig, size_t maxlen)
     strlcpy(orig, a->replacement, maxlen);
     return (0);
   } else {
-    perform_complex_alias(&d->input, ptr, a, d->character);
+    if (perform_complex_alias(&d->input, ptr, a) < 0)
+      send_to_char(d->character, "Alias expansion too long.\r\n");
     return (1);
   }
 }

tests/test_interpreter.c

@@ -2,6 +2,7 @@
  * @file test_interpreter.c
  * Unit tests for pure string-handling functions in src/interpreter.c:
  *   is_number, is_abbrev, delete_doubledollar, any_one_arg, one_word
+ * and for the alias expansion path perform_complex_alias() (via perform_alias()).
  */
 
 #include "unity.h"
@@ -10,6 +11,7 @@
 #include "sysdep.h"
 #include "structs.h"
 #include "utils.h"
+#include "comm.h"
 #include "interpreter.h"
 
 extern FILE *logfile;
@@ -17,6 +19,28 @@ extern FILE *logfile;
 void setUp(void)    { logfile = stderr; }
 void tearDown(void) { logfile = NULL; }
 
+/* =========================================================
+ * write_to_q — real implementation for alias tests.
+ * Overrides the __attribute__((weak)) stub in test_stubs.c so that
+ * the queue is actually populated and we can inspect its contents.
+ * ========================================================= */
+
+void write_to_q(const char *txt, struct txt_q *queue, int aliased)
+{
+    struct txt_block *newt;
+    CREATE(newt, struct txt_block, 1);
+    newt->text    = strdup(txt);
+    newt->aliased = aliased;
+    if (!queue->head) {
+        newt->next  = NULL;
+        queue->head = queue->tail = newt;
+    } else {
+        queue->tail->next = newt;
+        queue->tail       = newt;
+        newt->next        = NULL;
+    }
+}
+
 /* =========================================================
  * is_number
  * ========================================================= */
@@ -207,6 +231,219 @@ void test_one_word_leading_spaces_skipped(void)
     TEST_ASSERT_EQUAL_STRING("hello", first);
 }
 
+/* =========================================================
+ * Helpers for perform_complex_alias tests
+ * ========================================================= */
+
+/* Release every txt_block in a queue and reset head/tail to NULL. */
+static void free_txt_q(struct txt_q *q)
+{
+    struct txt_block *b = q->head, *n;
+    while (b) { n = b->next; free(b->text); free(b); b = n; }
+    q->head = q->tail = NULL;
+}
+
+/* Build a heap-allocated alias_data with type ALIAS_COMPLEX. */
+static struct alias_data *make_test_alias(const char *name, const char *repl)
+{
+    struct alias_data *a;
+    CREATE(a, struct alias_data, 1);
+    a->alias       = strdup(name);
+    a->replacement = strdup(repl);
+    a->type        = ALIAS_COMPLEX;
+    a->next        = NULL;
+    return a;
+}
+
+/* Free a single alias_data allocated by make_test_alias(). */
+static void destroy_test_alias(struct alias_data *a)
+{
+    free(a->alias);
+    free(a->replacement);
+    free(a);
+}
+
+/* Zero-initialise all three objects and wire them together so that
+ * IS_NPC() returns false and GET_ALIASES() returns a. */
+static void alias_env_init(struct descriptor_data     *d,
+                           struct char_data            *ch,
+                           struct player_special_data  *psd,
+                           struct alias_data           *a)
+{
+    memset(d,   0, sizeof(*d));
+    memset(ch,  0, sizeof(*ch));
+    memset(psd, 0, sizeof(*psd));
+    ch->player_specials = psd;
+    psd->aliases        = a;
+    d->character        = ch;
+}
+
+/* =========================================================
+ * perform_complex_alias — tested via the public perform_alias()
+ * ========================================================= */
+
+/* Literal replacement with no variable tokens. */
+void test_pca_literal(void)
+{
+    struct alias_data         *a = make_test_alias("lit", "hello world");
+    struct descriptor_data     d;
+    struct char_data           ch;
+    struct player_special_data psd;
+    char orig[] = "lit";
+
+    alias_env_init(&d, &ch, &psd, a);
+    perform_alias(&d, orig, sizeof(orig));
+
+    TEST_ASSERT_NOT_NULL(d.input.head);
+    TEST_ASSERT_EQUAL_STRING("hello world", d.input.head->text);
+    TEST_ASSERT_NULL(d.input.head->next);
+
+    free_txt_q(&d.input);
+    destroy_test_alias(a);
+}
+
+/* $* glob expands to the full argument string after the alias trigger. */
+void test_pca_glob_expansion(void)
+{
+    struct alias_data         *a = make_test_alias("say", "say $*");
+    struct descriptor_data     d;
+    struct char_data           ch;
+    struct player_special_data psd;
+    char orig[] = "say hello there";
+
+    alias_env_init(&d, &ch, &psd, a);
+    perform_alias(&d, orig, sizeof(orig));
+
+    TEST_ASSERT_NOT_NULL(d.input.head);
+    TEST_ASSERT_EQUAL_STRING("say hello there", d.input.head->text);
+
+    free_txt_q(&d.input);
+    destroy_test_alias(a);
+}
+
+/* $1 expands to the first whitespace-delimited token of the arguments. */
+void test_pca_token1_expansion(void)
+{
+    struct alias_data         *a = make_test_alias("s1", "say $1");
+    struct descriptor_data     d;
+    struct char_data           ch;
+    struct player_special_data psd;
+    char orig[] = "s1 hello world";
+
+    alias_env_init(&d, &ch, &psd, a);
+    perform_alias(&d, orig, sizeof(orig));
+
+    TEST_ASSERT_NOT_NULL(d.input.head);
+    TEST_ASSERT_EQUAL_STRING("say hello", d.input.head->text);
+
+    free_txt_q(&d.input);
+    destroy_test_alias(a);
+}
+
+/* Semicolon separator (;) produces two independent queue entries. */
+void test_pca_separator(void)
+{
+    struct alias_data         *a = make_test_alias("seq", "go north;go east");
+    struct descriptor_data     d;
+    struct char_data           ch;
+    struct player_special_data psd;
+    char orig[] = "seq";
+
+    alias_env_init(&d, &ch, &psd, a);
+    perform_alias(&d, orig, sizeof(orig));
+
+    TEST_ASSERT_NOT_NULL(d.input.head);
+    TEST_ASSERT_EQUAL_STRING("go north", d.input.head->text);
+    TEST_ASSERT_NOT_NULL(d.input.head->next);
+    TEST_ASSERT_EQUAL_STRING("go east", d.input.head->next->text);
+
+    free_txt_q(&d.input);
+    destroy_test_alias(a);
+}
+
+/* $$ in the replacement is preserved as $$ in the output (act-safety doubling). */
+void test_pca_dollar_dollar(void)
+{
+    struct alias_data         *a = make_test_alias("dol", "cost $$5");
+    struct descriptor_data     d;
+    struct char_data           ch;
+    struct player_special_data psd;
+    char orig[] = "dol";
+
+    alias_env_init(&d, &ch, &psd, a);
+    perform_alias(&d, orig, sizeof(orig));
+
+    TEST_ASSERT_NOT_NULL(d.input.head);
+    TEST_ASSERT_EQUAL_STRING("cost $$5", d.input.head->text);
+
+    free_txt_q(&d.input);
+    destroy_test_alias(a);
+}
+
+/* Overflow via $*: 255 "$*" tokens × 50-char argument exceeds
+ * MAX_RAW_INPUT_LENGTH.  The queue must be empty after the call. */
+void test_pca_overflow_glob(void)
+{
+    /* 255 × "$*" = 510 bytes + NUL */
+    char repl[511];
+    char orig[64];
+    struct alias_data         *a;
+    struct descriptor_data     d;
+    struct char_data           ch;
+    struct player_special_data psd;
+    int i;
+
+    for (i = 0; i < 255; i++) { repl[i * 2] = '$'; repl[i * 2 + 1] = '*'; }
+    repl[510] = '\0';
+
+    /* "al " + 50 'x' chars */
+    memcpy(orig, "al ", 3);
+    memset(orig + 3, 'x', 50);
+    orig[53] = '\0';
+
+    a = make_test_alias("al", repl);
+    alias_env_init(&d, &ch, &psd, a);
+    perform_alias(&d, orig, sizeof(orig));
+
+    /* No partial results must leak into the queue on overflow. */
+    TEST_ASSERT_NULL(d.input.head);
+
+    free_txt_q(&d.input);
+    destroy_test_alias(a);
+}
+
+/* Overflow via $1: 255 "$1" tokens × 50-char first token exceeds
+ * MAX_RAW_INPUT_LENGTH.  The queue must be empty after the call. */
+void test_pca_overflow_token(void)
+{
+    /* 255 × "$1" = 510 bytes + NUL */
+    char repl[511];
+    char orig[64];
+    struct alias_data         *a;
+    struct descriptor_data     d;
+    struct char_data           ch;
+    struct player_special_data psd;
+    int i;
+
+    for (i = 0; i < 255; i++) { repl[i * 2] = '$'; repl[i * 2 + 1] = '1'; }
+    repl[510] = '\0';
+
+    /* "al " + 50 'y' chars */
+    memcpy(orig, "al ", 3);
+    memset(orig + 3, 'y', 50);
+    orig[53] = '\0';
+
+    a = make_test_alias("al", repl);
+    alias_env_init(&d, &ch, &psd, a);
+    perform_alias(&d, orig, sizeof(orig));
+
+    /* No partial results must leak into the queue on overflow. */
+    TEST_ASSERT_NULL(d.input.head);
+
+    free_txt_q(&d.input);
+    destroy_test_alias(a);
+}
+
 /* =========================================================
  * main
  * ========================================================= */
@@ -252,5 +489,14 @@ int main(void)
     RUN_TEST(test_one_word_empty_quoted);
     RUN_TEST(test_one_word_leading_spaces_skipped);
 
+    /* perform_complex_alias */
+    RUN_TEST(test_pca_literal);
+    RUN_TEST(test_pca_glob_expansion);
+    RUN_TEST(test_pca_token1_expansion);
+    RUN_TEST(test_pca_separator);
+    RUN_TEST(test_pca_dollar_dollar);
+    RUN_TEST(test_pca_overflow_glob);
+    RUN_TEST(test_pca_overflow_token);
+
     return UNITY_END();
 }