Fix stack buffer overflow in var_subst() (dg_variables.c)

Agent-Logs-Url: https://github.com/tbamud/tbamud/sessions/5e320127-5bfa-499e-9776-e8a6cdd06bde

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

.github/workflows/build.yml

@@ -8,6 +8,8 @@ on:
 
 permissions:
   contents: read
+  checks: write
+  pull-requests: write
 
 jobs:
   build:
@@ -19,4 +21,15 @@ jobs:
     - name: configure
       run: ./configure
     - name: build
-      run: cd src && touch .accepted && make
+      working-directory: src
+      run: touch .accepted && make
+    - name: test
+      working-directory: tests
+      run: make test
+    - name: publish test results
+      uses: dorny/test-reporter@v1
+      if: always()
+      with:
+        name: Unity Tests
+        path: tests/test-results/*.xml
+        reporter: java-junit

.gitignore

@@ -7,6 +7,7 @@ config.status
 src/Makefile
 src/conf.h
 src/util/Makefile
+tests/Makefile
 src/.accepted
 src/depend
 src/util/depend
@@ -99,3 +100,4 @@ tests/test_utils
 tests/test_random
 tests/test_interpreter
 tests/test_class
+tests/test-results/

README.md

@@ -1,3 +1,34 @@
 Files for tbaMUD.
 
+## Unit Tests
+
+tbaMUD ships with a C unit-test suite located in the `tests/` directory.
+The suite uses the [Unity](https://github.com/ThrowTheSwitch/Unity) test
+framework (vendored under `tests/vendor/unity/`).
+
+### Quick start
+
+```
+./configure
+cd tests && make test
+```
+
+`make test` builds each test binary, runs it, and writes JUnit XML results to
+`tests/test-results/`.  A summary is printed to the terminal:
+
+```
+[PASS] test_utils
+[PASS] test_random
+[PASS] test_interpreter
+[PASS] test_class
+```
+
+### CI
+
+The GitHub Actions workflow (`.github/workflows/build.yml`) runs `make test`
+on every push and pull request against `master` and publishes a formatted
+report via the `dorny/test-reporter` action.
+
+See [doc/testing.md](doc/testing.md) for full details on adding new tests and
+understanding the test infrastructure.
 

doc/testing.md

@@ -0,0 +1,150 @@
+# tbaMUD Unit Testing
+
+_Updated 2026-04_
+
+## Overview
+
+tbaMUD has a C unit-test suite built on the
+[Unity](https://github.com/ThrowTheSwitch/Unity) framework.  Tests live in the
+`tests/` directory alongside the vendored Unity source.
+
+```
+tests/
+  Makefile.in          – Autoconf template; processed by configure
+  test_stubs.c         – Weak-symbol stubs that satisfy mud headers
+  unity_to_junit.py    – Converts Unity output to JUnit XML
+  test_class.c         – Tests for src/class.c
+  test_interpreter.c   – Tests for src/interpreter.c
+  test_random.c        – Tests for src/random.c
+  test_utils.c         – Tests for src/utils.c
+  vendor/unity/        – Vendored Unity test framework
+```
+
+## Prerequisites
+
+| Requirement | Notes |
+|---|---|
+| C compiler (gcc or clang) | Same compiler used to build the mud |
+| GNU make | Any POSIX-compatible make works |
+| Python 3 | Required only for JUnit XML conversion (`unity_to_junit.py`) |
+| autoconf / configure | Already needed to build the mud |
+
+## Running the tests
+
+Run `./configure` from the repository root first (only needed once):
+
+```sh
+./configure
+```
+
+Then build and run all tests from the `tests/` directory:
+
+```sh
+cd tests
+make test
+```
+
+`make test` performs the following steps for each test binary:
+
+1. Compiles the test binary (if not already up to date).
+2. Runs the binary and captures stdout/stderr to `test-results/<name>.out`.
+3. Measures wall-clock elapsed time.
+4. Converts the Unity output to JUnit XML via `unity_to_junit.py`, writing
+   `test-results/<name>.xml`.
+5. Prints `[PASS] <name>` or `[FAIL] <name>` and exits non-zero if any
+   binary failed.
+
+To build the test binaries without running them:
+
+```sh
+cd tests
+make
+```
+
+To remove all test binaries and result files:
+
+```sh
+cd tests
+make clean
+```
+
+## Test suites
+
+| Binary | Source under test | Test file |
+|---|---|---|
+| `test_utils` | `src/utils.c`, `src/random.c` | `test_utils.c` |
+| `test_random` | `src/random.c`, `rand_number`/`dice` in `src/utils.c` | `test_random.c` |
+| `test_interpreter` | `src/interpreter.c` | `test_interpreter.c` |
+| `test_class` | `src/class.c` | `test_class.c` |
+
+## Writing a new test
+
+### Adding a test case to an existing suite
+
+1. Open the relevant `test_<name>.c` file.
+2. Write a function with the signature `void test_my_feature(void)`.
+3. Use Unity assertion macros such as `TEST_ASSERT_EQUAL_INT`,
+   `TEST_ASSERT_NULL`, `TEST_ASSERT_TRUE`, etc.
+4. Register the function in the `main()` block:
+   ```c
+   RUN_TEST(test_my_feature);
+   ```
+
+Example:
+
+```c
+void test_str_cmp_equal_strings(void)
+{
+    TEST_ASSERT_EQUAL_INT(0, str_cmp("hello", "hello"));
+}
+```
+
+### Creating a new test suite
+
+1. Create `tests/test_<module>.c`.  Copy the boilerplate from an existing
+   suite: include `unity.h`, define `setUp`/`tearDown` (may be empty), write
+   test functions, and provide a `main()` that calls `UNITY_BEGIN()`,
+   `RUN_TEST(...)` for each function, and `return UNITY_END();`.
+
+2. Add the binary to `tests/Makefile.in`:
+   - Add the name to the `TESTS` variable.
+   - Add a build rule:
+     ```make
+     test_<module>: $(UNITY_SRC) $(STUBS_SRC) $(UTILS_SRC) \
+                    $(SRCDIR)/<module>.c test_<module>.c
+         $(COMPILE) -o $@ $^ $(LIBS)
+     ```
+
+3. Re-run `./configure` from the repository root to regenerate
+   `tests/Makefile` from the updated `tests/Makefile.in`.
+
+### Stubs
+
+Many mud source files reference global variables and functions that are only
+meaningful at runtime (e.g. `descriptor_list`, `log()`).  `test_stubs.c`
+provides zero-initialised definitions and `__attribute__((weak))` stub
+implementations for these symbols so that test binaries link without pulling
+in the full mud.
+
+If a new test requires a function not yet stubbed, add a weak stub to
+`test_stubs.c`:
+
+```c
+__attribute__((weak)) void my_function(void) { /* no-op */ }
+```
+
+## JUnit XML output and CI
+
+`unity_to_junit.py` reads Unity's line-oriented output on stdin and writes a
+JUnit-compatible XML file.  It accepts an optional third argument with the
+elapsed wall-clock time in seconds (provided by the `make test` target):
+
+```
+usage: unity_to_junit.py <suite_name> <output.xml> [elapsed_seconds]
+```
+
+The GitHub Actions workflow (`.github/workflows/build.yml`) runs `make test`
+on every push and pull request against `master`.  After the tests finish the
+`dorny/test-reporter` action reads `tests/test-results/*.xml` and publishes a
+formatted report as a GitHub Check with pass/fail counts and per-suite
+execution times.

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_variables.c

@@ -1636,6 +1636,13 @@ void var_subst(void *go, struct script_data *sc, trig_data *trig,
   int paren_count = 0;
   int dots = 0;
 
+  /* reject lines that would overflow our fixed-size buffers */
+  if (strnlen(line, MAX_INPUT_LENGTH) >= MAX_INPUT_LENGTH) {
+    script_log("Trigger VNum %d: variable substitution line too long, ignoring.", GET_TRIG_VNUM(trig));
+    *buf = '\0';
+    return;
+  }
+
   /* skip out if no %'s */
   if (!strchr(line, '%')) {
     strcpy(buf, line);

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;
         }
     }

tests/CMakeLists.txt

@@ -34,11 +34,14 @@ set(TEST_INCLUDES
 
 # Suppress warnings that fire in generated stubs / vendored code and in the
 # mud sources when compiled outside their normal full-build context.
-set(TEST_CFLAGS
+set(TEST_CFLAGS)
+if(CMAKE_C_COMPILER_ID MATCHES "^(GNU|Clang|AppleClang)$")
+    list(APPEND TEST_CFLAGS
         -Wno-unused-parameter
         -Wno-unused-function
         -Wno-unused-variable
     )
+endif()
 
 # Helper macro: add_mud_test(name SRC1 [SRC2 …])
 # Creates an executable, registers it with CTest.
@@ -81,8 +84,13 @@ add_mud_test(test_interpreter
     ${CMAKE_CURRENT_SOURCE_DIR}/test_interpreter.c
 )
 
-# crypt() is referenced from interpreter.c (nanny password hashing)
-target_link_libraries(test_interpreter PRIVATE crypt)
+# crypt() is referenced from interpreter.c (nanny password hashing).
+# Reuse the crypt library detected by the top-level build when one is needed;
+# on platforms where crypt() is provided by libc, no extra link library is
+# required.
+if(CRYPT_LIBRARY)
+    target_link_libraries(test_interpreter PRIVATE ${CRYPT_LIBRARY})
+endif()
 
 # ---------------------------------------------------------------------------
 # test_class — covers src/class.c

tests/Makefile

@@ -1,82 +0,0 @@
-# Generated automatically from Makefile.in by configure.
-# tests/Makefile.in
-# Autoconf template — processed by configure to produce tests/Makefile.
-#
-# Build and run the tbaMUD unit-test suite.
-# Usage (after running ./configure from the project root):
-#
-#   cd tests && make        # build all test binaries
-#   cd tests && make test   # build and run all tests
-
-CC        = gcc
-MYFLAGS   = -Wall -Wno-char-subscripts -Wno-unused-but-set-variable
-CFLAGS    = -g -O2 $(MYFLAGS)
-LIBS      =  -lcrypt 
-
-SRCDIR    = ../src
-UNITYDIR  = vendor/unity
-
-# Include paths:
-#   ../src   — mud headers and the generated conf.h
-#   vendor/unity — Unity framework headers
-INCFLAGS  = -I$(SRCDIR) -I$(UNITYDIR)
-
-# Suppress warnings that fire in generated stubs / vendored code
-WARNFLAGS = -Wno-unused-parameter -Wno-unused-function -Wno-unused-variable
-
-COMPILE   = $(CC) $(CFLAGS) $(WARNFLAGS) $(INCFLAGS)
-
-# Common object files compiled into every test binary
-UNITY_SRC  = $(UNITYDIR)/unity.c
-STUBS_SRC  = test_stubs.c
-
-# tbaMUD source files used by the tests
-UTILS_SRC  = $(SRCDIR)/utils.c $(SRCDIR)/random.c
-
-# All test binaries
-TESTS = test_utils test_random test_interpreter test_class
-
-.PHONY: all test clean
-
-all: $(TESTS)
-
-# ---------------------------------------------------------------------------
-# test_utils — covers src/utils.c
-# ---------------------------------------------------------------------------
-test_utils: $(UNITY_SRC) $(STUBS_SRC) $(UTILS_SRC) test_utils.c
-	$(COMPILE) -o $@ $^ $(LIBS)
-
-# ---------------------------------------------------------------------------
-# test_random — covers src/random.c and rand_number/dice in src/utils.c
-# ---------------------------------------------------------------------------
-test_random: $(UNITY_SRC) $(STUBS_SRC) $(UTILS_SRC) test_random.c
-	$(COMPILE) -o $@ $^ $(LIBS)
-
-# ---------------------------------------------------------------------------
-# test_interpreter — covers string helpers in src/interpreter.c
-# ---------------------------------------------------------------------------
-test_interpreter: $(UNITY_SRC) $(STUBS_SRC) $(UTILS_SRC) \
-                  $(SRCDIR)/interpreter.c test_interpreter.c
-	$(COMPILE) -o $@ $^ $(LIBS)
-
-# ---------------------------------------------------------------------------
-# test_class — covers src/class.c
-# ---------------------------------------------------------------------------
-test_class: $(UNITY_SRC) $(STUBS_SRC) $(UTILS_SRC) \
-            $(SRCDIR)/class.c test_class.c
-	$(COMPILE) -o $@ $^ $(LIBS)
-
-# ---------------------------------------------------------------------------
-# Run all tests
-# ---------------------------------------------------------------------------
-test: $(TESTS)
-	@echo "=========================================="
-	@echo "Running tbaMUD unit tests"
-	@echo "=========================================="
-	@./test_utils   && echo "[PASS] test_utils"   || echo "[FAIL] test_utils"
-	@./test_random  && echo "[PASS] test_random"  || echo "[FAIL] test_random"
-	@./test_interpreter && echo "[PASS] test_interpreter" || echo "[FAIL] test_interpreter"
-	@./test_class   && echo "[PASS] test_class"   || echo "[FAIL] test_class"
-
-clean:
-	rm -f $(TESTS)

tests/Makefile.in

@@ -66,16 +66,31 @@ test_class: $(UNITY_SRC) $(STUBS_SRC) $(UTILS_SRC) \
 	$(COMPILE) -o $@ $^ $(LIBS)
 
 # ---------------------------------------------------------------------------
-# Run all tests
+# Run all tests and produce JUnit XML files in test-results/
 # ---------------------------------------------------------------------------
 test: $(TESTS)
 	@echo "=========================================="
 	@echo "Running tbaMUD unit tests"
 	@echo "=========================================="
-	@./test_utils   && echo "[PASS] test_utils"   || echo "[FAIL] test_utils"
-	@./test_random  && echo "[PASS] test_random"  || echo "[FAIL] test_random"
-	@./test_interpreter && echo "[PASS] test_interpreter" || echo "[FAIL] test_interpreter"
-	@./test_class   && echo "[PASS] test_class"   || echo "[FAIL] test_class"
+	@mkdir -p test-results
+	@status=0; \
+	for t in $(TESTS); do \
+		t_start=$$(date +%s%3N); \
+		./$$t > test-results/$$t.out 2>&1; \
+		rc=$$?; \
+		t_end=$$(date +%s%3N); \
+		elapsed=$$(awk "BEGIN{printf \"%.3f\", ($$t_end - $$t_start)/1000}"); \
+		cat test-results/$$t.out; \
+		python3 unity_to_junit.py $$t test-results/$$t.xml "$$elapsed" < test-results/$$t.out; \
+		if [ $$rc -eq 0 ]; then \
+			echo "[PASS] $$t"; \
+		else \
+			echo "[FAIL] $$t"; \
+			status=1; \
+		fi; \
+	done; \
+	exit $$status
 
 clean:
 	rm -f $(TESTS)
+	rm -rf test-results

tests/test_random.c

@@ -24,7 +24,7 @@ void tearDown(void) { logfile = NULL; }
  * Precomputed for seed=1:
  *   call 1 → 16807
  *   call 2 → 282475249
- *   call 3 → 1622136673
+ *   call 3 → 1622650073
  * ========================================================= */
 
 void test_circle_random_deterministic_first(void)

tests/unity_to_junit.py

@@ -0,0 +1,80 @@
+#!/usr/bin/env python3
+"""Convert Unity test-runner output to JUnit XML.
+
+Usage:
+    ./test_binary | python3 unity_to_junit.py <suite_name> <output.xml> [elapsed_seconds]
+
+Unity emits one result line per test:
+    path/to/file.c:LINE:TEST_NAME:PASS
+    path/to/file.c:LINE:TEST_NAME:FAIL:message
+    path/to/file.c:LINE:TEST_NAME:IGNORE:message
+followed by a summary line:
+    N Tests N Failures N Ignored
+"""
+
+import re
+import sys
+import xml.etree.ElementTree as ET
+
+
+def parse_unity(lines):
+    tests = []
+    total = failures = ignored = 0
+    for line in lines:
+        line = line.rstrip("\n")
+        m = re.match(
+            r"^.+:\d+:([^:]+):(PASS|FAIL|IGNORE)(?::(.*))?$", line
+        )
+        if m:
+            name, result, message = m.group(1), m.group(2), m.group(3) or ""
+            tests.append((name, result, message))
+            continue
+        m2 = re.match(r"^(\d+) Tests (\d+) Failures (\d+) Ignored", line)
+        if m2:
+            total, failures, ignored = int(m2.group(1)), int(m2.group(2)), int(m2.group(3))
+    if not total:
+        total = len(tests)
+        failures = sum(1 for _, r, _ in tests if r == "FAIL")
+        ignored = sum(1 for _, r, _ in tests if r == "IGNORE")
+    return tests, total, failures, ignored
+
+
+def build_xml(suite_name, tests, total, failures, ignored, elapsed):
+    # Distribute total time evenly across tests for per-testcase timing.
+    per_test = round(elapsed / total, 6) if total else 0.0
+    suite = ET.Element(
+        "testsuite",
+        name=suite_name,
+        tests=str(total),
+        failures=str(failures),
+        errors="0",
+        skipped=str(ignored),
+        time=f"{elapsed:.6f}",
+    )
+    for name, result, message in tests:
+        case = ET.SubElement(
+            suite, "testcase",
+            name=name, classname=suite_name, time=f"{per_test:.6f}",
+        )
+        if result == "FAIL":
+            f = ET.SubElement(case, "failure", message=message)
+            f.text = message
+        elif result == "IGNORE":
+            ET.SubElement(case, "skipped", message=message)
+    return ET.ElementTree(suite)
+
+
+def main():
+    if len(sys.argv) < 3 or len(sys.argv) > 4:
+        print(f"usage: {sys.argv[0]} <suite_name> <output.xml> [elapsed_seconds]", file=sys.stderr)
+        sys.exit(1)
+    suite_name, output_file = sys.argv[1], sys.argv[2]
+    elapsed = float(sys.argv[3]) if len(sys.argv) == 4 else 0.0
+    tests, total, failures, ignored = parse_unity(sys.stdin.readlines())
+    tree = build_xml(suite_name, tests, total, failures, ignored, elapsed)
+    ET.indent(tree, space="  ")
+    tree.write(output_file, encoding="unicode", xml_declaration=True)
+
+
+if __name__ == "__main__":
+    main()