• cmdstr() can overflow a caller-supplied output buffer (@TYPE:, @INCLUD

    From Rob Swindell@1:103/705 to GitLab issue in main/sbbs on Tue Jul 21 00:24:25 2026
    open https://gitlab.synchro.net/main/sbbs/-/issues/1191

    ## Summary

    `sbbs_t::cmdstr()` bounds its output by the size of its *own member* buffer even
    when it is writing into a **caller-supplied** buffer whose size it is never told. Callers that pass a buffer smaller than `cmdstr_output` can therefore be overflowed. `@TYPE:` and `@INCLUDE:` do exactly that, into a 128- or 256-byte stack buffer.

    ## The code

    `src/sbbs3/xtrn.cpp`, `sbbs_t::cmdstr()`:

    ```c
    if (outstr == NULL)
    cmd = cmdstr_output; // member: char cmdstr_output[512] (sbbs.h)
    else
    cmd = outstr; // CALLER's buffer -- size unknown here
    len = strlen(instr);
    int maxlen = (int)sizeof(cmdstr_output) - 1; // 511, ALWAYS
    for (i = j = 0; i < len && j < maxlen; i++) {
    ...
    strncat(cmd, <expansion>, avail); // avail = maxlen - j
    ...
    }
    cmd[j] = 0;
    ```

    `maxlen` is derived from `cmdstr_output` unconditionally, so when `outstr` is supplied the loop and every `strncat(..., avail)` inside it are bounded by 511 rather than by the destination's real size.

    ## Reachable path

    `src/sbbs3/atcodes.cpp`, inside `sbbs_t::atcode()`:

    ```c
    if (!strncmp(sp, "TYPE:", 5))
    printfile(cmdstr(sp + 5, nulstr, nulstr, str), P_MODS);
    if (!strncmp(sp, "INCLUDE:", 8))
    printfile(cmdstr(sp + 8, nulstr, nulstr, str), P_OPENCLOSE | P_NOCRLF | P_SAVEATR | P_MODS);
    ```

    `str` there is `atcode()`'s output parameter. Its two callers pass:

    | caller | buffer | size |
    |---|---|---|
    | `show_atcode()` (`atcodes.cpp:181`) | `str2` | **128** |
    | `formatted_atcode()` (`atcodes.cpp:350`) | `buf` | **256** |

    So `cmdstr()` may write up to 512 bytes into a 128-byte stack buffer — roughly
    384 bytes past the end.

    Note the information is *available and discarded*: `atcode()` receives a `size_t maxlen` for exactly this buffer, but `cmdstr()` has no parameter to receive it.

    `exec.cpp`'s call sites pass `uchar buf[1025]` and are not affected.

    ## Trigger

    A `text`/`menu` file containing a long `@TYPE:` or `@INCLUDE:`. That content is sysop-authored, so this is not a remote attack primitive — but the *expanded* length is partly user-influenced, since `cmdstr()` substitutes `%a` (alias), `%u`, `%z` and friends, and a single line may repeat codes. A long path alone is sufficient with no user involvement.

    ## Also: silent truncation at 511

    Even where the destination is large enough, `cmdstr()` stops at 511 characters with no diagnostic, so an over-long configured command line is quietly shortened before `sbbs_t::external()` ever sees it. (`external()` now logs its own, separate truncation at `MAX_PATH+1`.)

    ## Suggested fix

    Two options, in increasing order of durability:

    1. **Enlarge the callers' buffers** to `sizeof(cmdstr_output)`. Removes the
    overflow with a one-line change per site, but leaves the underlying contract
    ("the caller must guess my member's size") intact and re-armed for the next
    caller.
    2. **Give `cmdstr()` an output size** — an added parameter, or a `cmdstr_len()`
    variant with the existing signature kept as a wrapper — and use that for
    `maxlen`. `atcode()` already has the right value to pass. Roughly 15 call
    sites, each a mechanical `sizeof` at the call site.

    (2) is the better long-term shape; (1) is a valid stopgap if a minimal change is wanted first.

    ## Status

    Found by code inspection while investigating an unrelated command-line truncation in `sbbs_t::external()`. **No crash has been demonstrated** — the bounds are read from the source, not from a reproducer, and I have not tried to construct one.

    — *Authored by Claude (Claude Code), on behalf of @rswindell*
    --- SBBSecho 3.37-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)