From af06fa0621ce3be4de5b7dbc2ccac7d9c65379ce Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Sat, 25 Apr 2026 20:59:04 +0200 Subject: [PATCH] Fix stack buffer overflow in perform_complex_alias() via unbounded alias expansion (#180) * Fix stack buffer overflow in perform_complex_alias() - add bounds checks * Refactor: use return value instead of char_data param in perform_complex_alias() * Add unit tests for perform_complex_alias() via perform_alias() Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: welcor <357770+welcor@users.noreply.github.com> --- src/interpreter.c | 39 +++++-- tests/test_interpreter.c | 246 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 277 insertions(+), 8 deletions(-) diff --git a/src/interpreter.c b/src/interpreter.c index 1bdaec8..0782ace 100644 --- a/src/interpreter.c +++ b/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); +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,9 +668,10 @@ ACMD(do_alias) * commands. */ #define NUM_TOKENS 9 -static void perform_complex_alias(struct txt_q *input_q, char *orig, struct alias_data *a) +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; char *tokens[NUM_TOKENS], *temp, *write_point; char buf2[MAX_RAW_INPUT_LENGTH], buf[MAX_RAW_INPUT_LENGTH]; /* raw? */ int num_of_tokens = 0, num; @@ -697,16 +698,27 @@ static void perform_complex_alias(struct txt_q *input_q, char *orig, struct alia } else if (*temp == ALIAS_VAR_CHAR) { temp++; if ((num = *temp - '1') < num_of_tokens && num >= 0) { - strcpy(write_point, tokens[num]); /* strcpy: OK */ + if ((write_point - buf) + strlen(tokens[num]) >= MAX_RAW_INPUT_LENGTH) + goto overflow; + strcpy(write_point, tokens[num]); write_point += strlen(tokens[num]); } else if (*temp == ALIAS_GLOB_CHAR) { skip_spaces(&orig); - strcpy(write_point, orig); /* strcpy: OK */ + if ((write_point - buf) + strlen(orig) >= MAX_RAW_INPUT_LENGTH) + goto overflow; + strcpy(write_point, orig); write_point += strlen(orig); - } else if ((*(write_point++) = *temp) == '$') /* redouble $ for act safety */ - *(write_point++) = '$'; - } else + } else { + if (write_point - buf + 2 >= MAX_RAW_INPUT_LENGTH) + goto overflow; + if ((*(write_point++) = *temp) == '$') /* redouble $ for act safety */ + *(write_point++) = '$'; + } + } else { + if (write_point - buf + 1 >= MAX_RAW_INPUT_LENGTH) + goto overflow; *(write_point++) = *temp; + } } *write_point = '\0'; @@ -720,6 +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 (0); + +overflow: + 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. @@ -755,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); + if (perform_complex_alias(&d->input, ptr, a) < 0) + send_to_char(d->character, "Alias expansion too long.\r\n"); return (1); } } diff --git a/tests/test_interpreter.c b/tests/test_interpreter.c index 230cf73..ee673ac 100644 --- a/tests/test_interpreter.c +++ b/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(); }