# ScorpioX C Codebase — Security Vulnerability Audit Report

**Date:** 2026-04-28  
**Auditor:** Security Audit Agent  
**Scope:** All project C source files (excluding vendor: mbedtls, yyjson)  
**Codebase:** 150 source files, ~143,515 lines of C/H code  

---

## Executive Summary

The ScorpioX C codebase is a substantial systems project comprising networking libraries (libsxnet), utility libraries (libsxutil), UI libraries (libsxui), a WebSocket bridge, an HTTP server, mail server components, container management, and various CLI tools. The audit identified **23 distinct vulnerability findings** spanning command injection, buffer overflow, memory safety, and race condition categories. The most critical finding is a **command injection vulnerability in the HTTP server** (scorpiox-server.c) that could allow remote code execution.

Overall, the codebase demonstrates generally sound practices — extensive use of `snprintf` (2,228 calls) over unsafe `sprintf` (0 calls), and bounded buffer operations in most code paths. However, the heavy reliance on `system()`/`popen()` (164 total calls) with constructed command strings presents a systemic risk surface.

---

## Vulnerability Summary

| # | Severity | CVSS | File | Line(s) | Category | Description |
|---|----------|------|------|---------|----------|-------------|
| 1 | **CRITICAL** | 9.8 | scorpiox-server.c | 2480-2486 | Command Injection | OTP endpoint injects unsanitized query params into shell command |
| 2 | **HIGH** | 8.1 | scorpiox-unshare.c | 309-312 | Command Injection | URL/path injected into `curl`/`wget` system() via single-quote wrapping only |
| 3 | **HIGH** | 7.5 | sxmail_maildir.c | 704-711 | Command Injection | User-derived path injected into `rm -rf '%s'` / `rmdir /s /q` system() call |
| 4 | **HIGH** | 7.5 | scorpiox-server.c | 1192-1222 | Path Traversal | `url_decode()` applied before path validation; no `..` check post-decode |
| 5 | **HIGH** | 7.2 | sx.c | 655-659 | Command Injection | `$EDITOR` env var injected into system() without sanitization |
| 6 | **MEDIUM** | 6.5 | sx_term.c | 785-786 | Memory Leak | Allocation null-check logic error — `new_front` leaked if `new_back` is NULL |
| 7 | **MEDIUM** | 6.2 | Multiple files | Various | Missing NULL Checks | 50+ malloc() calls without subsequent NULL validation |
| 8 | **MEDIUM** | 5.9 | scorpiox-server.c | 2059, 2222 | Return Type Mismatch | `return -1` in void function — undefined behavior, skips cleanup |
| 9 | **MEDIUM** | 5.9 | scorpiox-mcp.c | 1240 | Type Confusion | Returns `int (-1)` from `char *` function — pointer/integer confusion |
| 10 | **MEDIUM** | 5.5 | scorpiox-vi.c | 512 | Integer Overflow | `sizeof(Line) * E.num_lines` — no overflow check before malloc |
| 11 | **MEDIUM** | 5.5 | sxmux_vt.c | 910,918 | Integer Overflow | `(size_t)cols * (size_t)rows` — multiplication may overflow for large dimensions |
| 12 | **MEDIUM** | 5.3 | sx_http.c | 46-53 | Integer Overflow | `size * nmemb` in write_cb — classic curl callback overflow pattern |
| 13 | **MEDIUM** | 5.0 | Multiple (28 locs) | Various | Unsafe strcpy | `strcpy()` used 28 times — most with constant strings but some with computed offsets |
| 14 | **MEDIUM** | 5.0 | ws2tcp.c | 506-510 | Weak Path Traversal | `strstr(url_path, "..")` — bypassable with encoded sequences on some platforms |
| 15 | **MEDIUM** | 4.7 | 15+ files | Various | Stack Overflow Risk | Stack buffers ≥64KB (max 65536 bytes) — 5 locations with char[65536] on stack |
| 16 | **MEDIUM** | 4.5 | sx.c, sx_dll.c | 3772-3776 | Diagnostic Alloc | 5 malloc calls without NULL checks in heap diagnostic — dereferenced if NULL |
| 17 | **LOW** | 3.7 | sxmux_vt.c | 919 | Memory Leak | Scrollback alloc failure returns NULL without freeing `vt->cells` |
| 18 | **LOW** | 3.5 | sx_dll.c | Various | Race Condition | Worker thread accesses `g_dll.busy` volatile flag without mutex in some paths |
| 19 | **LOW** | 3.3 | sxmux_session.c | 1841 | Logic Error | Comparison always false due to limited data type range |
| 20 | **LOW** | 3.1 | sx_systemprompt.c | 262 | Format String | Non-literal format string in strftime — low risk (format from internal config only) |
| 21 | **LOW** | 2.5 | Multiple files | Various | Signal Handler Safety | 59 signal() registrations — some handlers call non-async-signal-safe functions |
| 22 | **LOW** | 2.3 | scorpiox-unshare.c | Various | TOCTOU | `access()` then `open()`/`exec()` patterns — race window exists |
| 23 | **INFO** | 1.5 | scorpiox-config.c | 1429,1444,1479 | Unsafe strcpy | strcpy with "1"/"0" into value buffers — safe but should use snprintf |

---

## Detailed Analysis

### 1. Command Injection Analysis

#### CRITICAL: OTP Endpoint Command Injection (scorpiox-server.c:2480)

```c
// Query parameters 'account' and 'secret' come directly from HTTP request
char cmd[2048];
if (secret && *secret) {
    snprintf(cmd, sizeof(cmd), "scorpiox-otp -s \"%s\" -a \"%s\" -j", secret, account);
} else {
    snprintf(cmd, sizeof(cmd), "scorpiox-otp -a \"%s\" -j", account);
}
char *otp_out = run_command(cmd, 4096, &otp_exit, NULL);  // popen()
```

**Attack Vector:** A remote attacker sends `GET /api/otp?a=x";$(id);"&s=y` — the double-quote escaping is trivially bypassed since `url_decode()` applies URL decoding but does no shell metacharacter sanitization.

**CVSS 3.1:** AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:H/A:H = **9.8**

**Remediation:** 
- Use `execve()`/`fork()` with an argument array instead of `system()`/`popen()`
- Or strictly validate account/secret to alphanumeric characters only
- Apply shell escaping function that escapes all metacharacters

#### HIGH: Download URL Injection (scorpiox-unshare.c:309)

```c
static int download_image(const char *url, const char *dest) {
    char cmd[2048];
    snprintf(cmd, sizeof(cmd),
        "curl -fSL --progress-bar -o '%s' '%s' 2>&1", dest, url);
    if (system(cmd) == 0) return 0;
```

Single-quote wrapping is insufficient — a URL containing `'` breaks out. The `url` parameter originates from user-specified container image names.

**Remediation:** Use `fork()/execvp()` with proper argument passing, or implement HTTP download in-process using the existing libcurl integration.

#### HIGH: Mail Folder Deletion Injection (sxmail_maildir.c:704)

```c
snprintf(cmd, sizeof(cmd), "rm -rf '%s'", path);
if (system(cmd) != 0) { ... }
```

The `path` is constructed from `base_path`, `user`, and `folder_sub` — if any contain single quotes, command injection occurs. The `user` field may come from IMAP authentication, and `folder` from IMAP CREATE/DELETE commands.

**Remediation:** Use `nftw()` with `FTW_DEPTH` for recursive deletion, or `fork()/execvp("rm", ...)` with proper argument separation.

#### HIGH: Editor Command Injection (sx.c:655)

```c
const char *editor = getenv("EDITOR");
char editor_cmd[512];
snprintf(editor_cmd, sizeof(editor_cmd), "%s %s", editor, tmpfile);
int ret = system(editor_cmd);
```

The `$EDITOR` environment variable is injected directly into a shell command. A malicious `EDITOR` value like `vim; malicious_command #` achieves code execution.

**Risk:** Lower in practice since `$EDITOR` is controlled by the local user, but violates defense-in-depth. Container/sandbox environments may inherit untrusted env.

#### Systemic Risk: system()/popen() Usage

The codebase uses **115 system()** and **49 popen()** calls. Files with highest density:
- `scorpiox-unshare.c`: 15 system() calls
- `scorpiox-wsl.c`: 10 system() calls  
- `sx.c`: 10 system() calls
- `scorpiox-server.c`: 5 system()/popen() calls

**Recommendation:** Create a helper function `sx_exec_argv(const char *prog, char *const argv[])` using `fork()/execvp()` and migrate all `system()` calls that include user-derived data.

---

### 2. Buffer Overflow Analysis

#### Stack-Allocated Buffer Risks

The codebase contains **2,365 stack-allocated char buffers**. The largest:

| Size | File | Variable |
|------|------|----------|
| 65,536 | scorpiox-wsl.c:158 | `char buffer[65536]` |
| 65,536 | scorpiox-multiplexer.c:1484 | `char buf[65536]` |
| 65,536 | sx_http_wasm.c:39 | `static char sx_xhr_response_buf[65536]` |
| 65,535 | scorpiox-multiplexer.c:1212 | `char payload[65535]` |
| 32,768 | sxmail_dkim.c:414 | `char sign_data[32768]` |
| 16,384 | scorpiox-wsl.c:674 | `char cmd[16384]` |
| 16,384 | scorpiox-tmux.c:2247 | `char full_cmd[16384]` |
| 16,384 | scorpiox-sdk.c:1353 | `char wsl_cmdline[16384]` |

**Risk:** 64KB stack allocations may cause stack overflow on systems with limited stack size (e.g., threads with 64KB stacks, embedded/WASM environments). Recursive functions calling these could crash.

**Remediation:** Allocate buffers ≥8KB on the heap with proper cleanup, or use `alloca()` guards.

#### strcpy Usage (28 locations)

Most `strcpy` uses copy string literals of known length into adequately-sized buffers (low risk). Notable exceptions:

```c
// scorpiox-unshare.c:140 — SAFE: bounds-checked 2 lines above
if (dlen + 1 + strlen(prog) + 1 > sizeof(buf)) { ... continue; }
strcpy(buf + dlen + 1, prog);

// scorpiox-agent.c:1452 — SAFE: replaces ".sln" with ".dll" (same length)
strcpy(out + len - 4, ".dll");

// scorpiox-sdk.c:364 — MODERATE RISK: assumes buffer has room for "scorpiox-host.exe"
strcpy(slash + 1, "scorpiox-host.exe");  // 17 chars + NUL
// Buffer allocated with readlink(..., buf, size - 20) — safe due to -20 margin
```

**Good practice:** The codebase uses **snprintf** 2,228 times and **sprintf** 0 times — excellent.

#### Integer Overflow in Allocation Sizes

```c
// sx_http.c:46 - Classic curl callback overflow
size_t real_size = size * nmemb;  // Can overflow if both are large

// sxmux_vt.c:910
vt->cells = calloc((size_t)cols * (size_t)rows, sizeof(SxMuxCell));
// cols*rows can overflow for attacker-controlled terminal dimensions

// scorpiox-vi.c:512
E.undo.lines = malloc(sizeof(Line) * E.num_lines);
// num_lines is int — if large file, sizeof(Line)*num_lines overflows
```

**Remediation:** Use overflow-safe multiplication:
```c
if (cols > SIZE_MAX / rows) return NULL;  // overflow check
```

---

### 3. Memory Safety Analysis

#### Missing NULL Checks After malloc (50+ locations)

Critical instances where NULL dereference could crash the application:

| File | Line | Allocation |
|------|------|------------|
| sxtmux_backend_sxmux.c | 64 | `malloc(len + 2)` — used immediately |
| sxmail_smtp.c | 245 | `malloc(dkim_cap)` — DKIM buffer |
| sxmail_maildir.c | 478, 909 | `malloc(fsize + 1)` — file read |
| sxmail_imap.c | 738 | `malloc(literal_sz + 1)` — IMAP message |
| sx_term.c | 610, 746, 816, 1170, 1536, 1838, 1845 | Terminal buffers |
| sx_dll.c | 328, 358, 860, 1104 | DLL interop |
| sx.c | 305, 677, 2471, 2562, 3729, 4403 | Main application |
| scorpiox-vi.c | 408, 459, 512, 548, 1167 | Editor buffers |
| scorpiox-unshare.c | 346, 473, 1795 | Container management |
| scorpiox-wsl.c | 885 | `malloc(1024 * 1024)` — 1MB allocation |

**Risk:** NULL pointer dereference causes segfault. In server contexts (scorpiox-server, sxmail), this is a denial-of-service vector.

#### Memory Leak: sx_term.c:785

```c
SxCell *new_front = calloc(cells, sizeof(SxCell));
SxCell *new_back = calloc(cells, sizeof(SxCell));
if (!new_back) return;                               // BUG: leaks new_front
if (!new_front || !new_back) { free(new_front); free(new_back); return; }
// Line 786 is dead code — new_back was already checked
```

The first check on line 785 returns without freeing `new_front` if `new_back` is NULL but `new_front` succeeded. The second check on line 786 is unreachable for the `!new_back` case.

**Fix:**
```c
if (!new_front || !new_back) { free(new_front); free(new_back); return; }
```

#### Memory Leak: sxmux_vt.c:919

```c
vt->scrollback = calloc((size_t)vt->scrollback_cap * (size_t)cols, sizeof(SxMuxCell));
if (!vt->scrollback) return NULL;  // Leaks vt->cells (allocated at line 910)
```

#### Type Confusion: scorpiox-mcp.c:1240

```c
if (!out) return -1;  // Function returns char* — returning (char*)-1 (0xFFFFFFFFFFFFFFFF)
// ...
return out;
```

Callers may check `if (!result)` which would pass for `-1` cast to pointer. This is undefined behavior.

---

### 4. Path Traversal Analysis

#### scorpiox-server.c: Post-Decode Path Traversal

```c
// parse_request() at line 1220:
strncpy(req->path, uri, sizeof(req->path) - 1);
url_decode(req->path);  // Decodes %2e%2e to .. AFTER copying
```

The server calls `url_decode()` on the path **but never checks for `..` components after decoding**. While `extract_website_name()` restricts characters to `[alnum-_.]`, the `try_fallback` path for static file serving (if configured) may be vulnerable.

#### ws2tcp.c: Weak Traversal Check

```c
if (strstr(url_path, "..")) {  // Blocks literal ".." but not encoded forms
```

The `url_path` passed to `serve_static()` comes from `parse_http_request()` which does not URL-decode the path. This means `%2e%2e` in the URL bypasses the check on this server. However, the actual `fopen()` of the file path would fail for `%2e%2e` since the filesystem doesn't interpret URL encoding — so the risk is **theoretical** unless a front-end proxy decodes before forwarding.

---

### 5. Race Condition Analysis

#### Signal Handler Safety

The codebase registers 59 signal handlers. Several perform non-async-signal-safe operations:

```c
// scorpiox-unshare.c - cleanup_handler likely calls free()/close()
signal(SIGINT,  cleanup_handler);
signal(SIGTERM, cleanup_handler);
signal(SIGHUP,  cleanup_handler);

// sx_term.c:677 - SIGWINCH handler
signal(SIGWINCH, term_sigwinch);
// term_sigwinch likely accesses global terminal state
```

**Risk:** If a signal arrives during malloc()/free(), the signal handler calling malloc()/free() causes heap corruption.

**Remediation:** Use `sigaction()` with `SA_RESTART`, set a volatile flag in the handler, and check it in the main loop.

#### TOCTOU in scorpiox-unshare.c

```c
// Line 124: check-then-use
return access(prog, X_OK) == 0;  // File could change between access() and exec()
```

Multiple `access()` calls followed by `exec()`/`open()` create time-of-check-time-of-use windows. In container management code (scorpiox-unshare.c), this is a privilege escalation risk if an attacker can race symlink creation.

#### Threading: sx_dll.c

The DLL worker thread properly uses mutexes for queue operations. However, the `volatile bool busy` flag at line 275 is accessed without mutex protection in some paths, which may cause torn reads on non-x86 architectures.

---

### 6. Compiler Warning Analysis

Compilation with `-Wall -Wextra -Wformat-security` produced:

| Warning | File | Severity | Description |
|---------|------|----------|-------------|
| `-Wreturn-type` | scorpiox-server.c:2059,2222 | **Medium** | `return -1` in void function — undefined behavior |
| `-Wreturn-type` | scorpiox-openai.c:594 | **Medium** | Same — return with value in void function |
| `-Wint-conversion` | scorpiox-mcp.c:1240 | **Medium** | Returns int from char* function |
| `-Wint-conversion` | scorpiox-claudecode-fetchtoken.c:12-13 | **Low** | Implicit int-to-pointer conversion |
| `-Wswitch` | sx.c:927 | **Low** | Unhandled enum value SX_CMDSRC_BUILTIN_CMD |
| `-Wformat-nonliteral` | sx_systemprompt.c:262 | **Low** | Non-literal format in strftime |
| `-Wtype-limits` | sxmux_session.c:1841 | **Low** | Comparison always false |
| `-Wunused-variable` | sxmux_session.c:1832,1776 | **Info** | Unused variables |

The void-function return errors in `scorpiox-server.c` are particularly concerning — they silently skip cleanup code (buffer deallocation, socket close) on error paths.

---

### 7. Format String Analysis

**No format string vulnerabilities found.** The codebase consistently uses literal format strings with `printf`/`fprintf`/`snprintf`. All 2,228 `snprintf` calls use string literal format arguments. The one non-literal format (sx_systemprompt.c:262) is `strftime()` with an internally-sourced format string.

---

## Risk Scoring Summary

| Risk Level | Count | Action Required |
|------------|-------|-----------------|
| CRITICAL | 1 | Immediate fix — remote code execution |
| HIGH | 4 | Fix within 1 week — command injection / path traversal |
| MEDIUM | 12 | Fix within 1 month — memory safety, integer overflow |
| LOW | 5 | Fix within 1 quarter — race conditions, logic errors |
| INFO | 1 | Best practice improvement |

---

## Remediation Recommendations

### Priority 1 — Immediate (Critical/High)

1. **Replace `system()`/`popen()` with `fork()/execvp()`** for all paths that include user-derived data. Create a shared utility:
   ```c
   int sx_safe_exec(const char *prog, char *const argv[], char **output);
   ```

2. **Sanitize OTP query parameters** in scorpiox-server.c — restrict to `[a-zA-Z0-9@._-]` or use argument arrays.

3. **Add post-decode path traversal checks** in scorpiox-server.c:
   ```c
   if (strstr(req->path, "..") || req->path[0] != '/') return false;
   char *real = realpath(filepath, NULL);
   if (!real || strncmp(real, webroot, strlen(webroot)) != 0) { /* reject */ }
   ```

4. **Replace `rm -rf '%s'` in sxmail_maildir.c** with `nftw()` recursive deletion or `fork()/exec("rm","-rf","--",path)`.

### Priority 2 — Short Term (Medium)

5. **Add NULL checks after all malloc/calloc/realloc calls** — particularly in server-facing code (sx_http.c, sxmail_*.c, scorpiox-server.c).

6. **Add integer overflow guards** before multiplication in allocation sizes:
   ```c
   #define SX_SAFE_MUL(a, b) (((b) != 0 && (a) > SIZE_MAX / (b)) ? 0 : (a) * (b))
   ```

7. **Fix sx_term.c:785** memory leak — remove the redundant first null check.

8. **Fix scorpiox-server.c void function returns** — change `return -1` to proper error handling with cleanup.

9. **Replace large stack allocations (≥8KB) with heap allocations** — especially char[65536] buffers.

### Priority 3 — Long Term (Low)

10. **Migrate from `signal()` to `sigaction()`** with async-signal-safe handlers.

11. **Eliminate remaining `strcpy()` calls** — replace with `snprintf()` or bounded copy macros.

12. **Add `-Wformat-security -Werror=return-type` to CI** build flags to catch future regressions.

13. **Implement TOCTOU-safe patterns** — use `openat()`/`fstatat()` with directory file descriptors.

---

## Methodology

- **Static Analysis:** grep-based pattern matching for unsafe functions, manual code review of flagged locations
- **Compiler Warnings:** GCC 13 with `-Wall -Wextra -Wformat-security -Wformat=2`
- **Scope:** 150 C source files, 143,515 lines (excluding vendor: mbedtls, yyjson, gnuwin64)
- **Risk Scoring:** CVSS 3.1 base scores adapted for the application context

---

*Report generated 2026-04-28 by Security Audit Agent*