c-audit

C Code Audit Skill

Perform comprehensive security, safety, and quality audits on Hull C code.

Target: $ARGUMENTS (default: all src/cap/, src/runtime/, and include/hull/ files)

Usage

/c-audit                              # Audit all source files
/c-audit src/cap/hull_cap_db.c        # Audit a specific file
/c-audit --fix                        # Audit and apply fixes

Audit Categories

1. Memory Safety (Critical)

Safe Replacements:

// Copying
strcpy(dst, src)           -> strncpy(dst, src, sizeof(dst)-1); dst[sizeof(dst)-1] = '\0';
strcat(dst, src)           -> strncat(dst, src, sizeof(dst)-strlen(dst)-1);

// Formatting
sprintf(buf, fmt, ...)     -> snprintf(buf, sizeof(buf), fmt, ...);
gets(buf)                  -> fgets(buf, sizeof(buf), stdin);

// Memory allocation (overflow-safe)
malloc(count * size)       -> calloc(count, size);

// Integer parsing (atoi/atol have no error detection!)
atoi(str)                  -> strtol(str, &end, 10) with validation
atof(str)                  -> strtof(str, &end) with validation

Allocator Discipline:

// Hull uses malloc/free directly (no custom allocator like Keel)
// Verify: every malloc has a matching free
// Verify: every malloc return is checked for NULL
// Verify: calloc used when count*size multiplication is needed

// BAD:
void *p = malloc(count * elem_size);  // overflow risk

// GOOD:
void *p = calloc(count, elem_size);   // overflow-safe
if (!p) return -1;

2. Input Validation

3. Resource Management

Hull Cleanup Pattern:

// Every init must have matching free
hl_js_init()           -> hl_js_free()
hl_lua_init()          -> hl_lua_free()
sqlite3_open()         -> sqlite3_close()
sqlite3_prepare_v2()   -> sqlite3_finalize()
JS_NewRuntime()        -> JS_FreeRuntime()
JS_NewContext()         -> JS_FreeContext()
lua_newstate()         -> lua_close()
fopen()                -> fclose()

4. Integer Overflow

Overflow in size computations can cause undersized allocations and buffer overflows.

// BAD: overflow on 32-bit
int total = count * sizeof(HlColumn);
void *buf = malloc(total);

// GOOD: use size_t + calloc
HlColumn *cols = calloc(count, sizeof(HlColumn));

// GOOD: check before multiply
if (count > 0 && (size_t)count > SIZE_MAX / sizeof(HlColumn)) {
    return -1;  // overflow
}

Key areas in Hull:

• HlColumn array allocation in hl_cap_db_query() row callbacks
• PBKDF2 output buffer sizing
• SQL parameter arrays (HlValue binding arrays)
• Filesystem path buffer construction (base_dir + "/" + relative_path)
• QuickJS/Lua value conversion arrays

5. Capability Boundary Enforcement

Hull's security model depends on the shared hl_cap_* layer. Verify:

5b. Sealed runtime tables (read-only memory protection)

Hull defends boot-built security policy via two mechanisms (see docs/security.md §4b):

• .rodata for compile-time constants (static const tables — automatic OS read-only via the linker).
• hl_seal_arena (page-backed mmap RW → mprotect RO) for data that can't be static const because it's built at boot from app input (manifest, configuration, capability resolution).

When auditing C runtime changes, look for these patterns:

Cross-platform guard. hl_seal_arena is POSIX (mmap/mprotect/ sysconf). Cosmopolitan provides the POSIX shim transparently. If you ever add a native MSVC build, the arena needs a #ifdef _WIN32 branch using VirtualAlloc / VirtualProtect — same API contract.

Death tests are non-negotiable. Any new sealed surface needs a fork+SIGSEGV test that writes to a sealed page from a child process and asserts the child died with SIGSEGV/SIGBUS. Without it, a no-op mprotect would silently pass every other test. Pattern in tests/hull/test_seal_arena.c::write_after_seal_faults.

6. Defensive Macros

Check for and suggest these patterns:

#define ARRAY_LEN(a) (sizeof(a) / sizeof((a)[0]))
#define SAFE_FREE(p) do { free(p); (p) = NULL; } while(0)
#define CLAMP(x, lo, hi) ((x) < (lo) ? (lo) : ((x) > (hi) ? (hi) : (x)))

7. Test Coverage

Check test files (tests/test_*.c) for:

• Basic functionality tests
• Edge cases (empty input, max values, NULL)
• Error path tests (what happens when things fail)
• Bounds checking tests
• All public API functions have at least one test
• Path traversal rejection tested
• SQL parameterization tested
• Env allowlist enforcement tested
• Sandbox restrictions tested (both JS and Lua)

8. Dead Code Detection

Compile with -Wunused flags to detect automatically.

9. Build hardening + ROP/JOP resistance

Hull ships a probe-based compiler/linker hardening layer (Makefile ~lines 69-180, full reference in docs/security.md §4c). The verifier at scripts/check_hardening.sh runs on every CI build and every release native target; the release fails if a required protection drops.

9.1. Verify the build flags are still applied

The baseline (unconditional on non-COSMO):

-fstack-protector-strong  -fPIE  [-pie on Linux]
-D_FORTIFY_SOURCE=3  (release only)
-Wl,-z,relro -Wl,-z,now -Wl,-z,noexecstack  (Linux only)

The probed set (added if the toolchain accepts each — older toolchains reject some; the probe at hl_have_cflag / hl_have_ldflag handles fallback cleanly):

CFLAGS:   -fstack-clash-protection
          -fno-plt  -fno-common
          -ftrivial-auto-var-init=zero
          -fzero-call-used-regs=used-gpr
          -fcf-protection=full         (x86_64 only)
          -mbranch-protection=standard (arm64 only)
LDFLAGS:  -Wl,-z,separate-code  (Linux only)
          -Wl,--as-needed       (universal where supported)

Sanity checks for any audit touching the Makefile:

• make hardening runs without error and reports the expected flags for the host toolchain.
• make check-hardening passes on macOS and Linux (skips everything on cosmo, expected).
• Existing -fstack-protector-strong / -fPIE / FORTIFY / RELRO baseline still present.
• If a new flag is added: probe it via hl_have_cflag rather than hard-applying — must not break older toolchains.
• HULL_DISABLE_HARDENING=1 opt-out still works for local debugging (and CI's check-hardening step still fails on release if it was set).

9.2. C-level patterns that DEFEAT the hardening

The hardening flags raise the cost of exploitation; specific C patterns can undo it in a single line. When auditing new code, flag these:

9.3. Sanitizer / debug-mode hygiene

9.4. Expected commands

• make debug — ASan + UBSan + -O0 -g -fno-omit-frame-pointer.
• make msan — MSan + UBSan (Linux clang only).
• make check — clean + ASan build + test + e2e.
• make hardening — print resolved hardening flag list.
• make check-hardening — run post-build verifier on build/hull.
• scripts/check_hardening.sh [BINARY] — same, on any binary.

Audit checklist for §9:

• No new RWX-introducing patterns (mmap+PROT_EXEC|PROT_WRITE, mprotect to W+X).
• No new dlopen/dlsym in Hull TUs.
• No new global function-pointer tables missing static const.
• No new unbounded alloca / VLAs.
• No new __attribute__((constructor)) touching dispatch state.
• make hardening reports the expected flag set.
• make check-hardening passes on macOS + Linux + cosmo.
• Sanitizer builds still build and pass.

10. Lifetime classes & arena discipline

Hull's trusted core avoids most use-after-free / double-free / lifetime bugs by construction: security-sensitive code does not individually free() objects, it lets an arena's destroy point free everything at once. When auditing allocation, classify every site by lifetime class and check it uses the right ownership model.

Patterns to flag:

10.1. realloc / internal-mutable-pointer hazards

Two specific sub-checks that catch the subtle lifetime bugs:

• Dangling-after-realloc. Grep every realloc. For each, confirm the result is stored back into the owning struct and that the code does not hold a pointer/iterator obtained before the realloc. Growable buffers (tui out_buf, worker_db value array, tool result array, agent line buffers, poll timer/watcher/completion arrays) are the usual sites. A bounded-size check (SIZE_MAX/2) must precede the doubling.
• API returning a pointer into mutable internal storage. A public hl_* getter must return either a const pointer into immutable .rodata/sealed memory (e.g. hl_vfs_find → const HlEntry *), a freshly-allocated string with a documented caller-frees contract, or a stable handle the caller is meant to drive. Flag any getter that hands back a non-const pointer into a buffer a later call could realloc/free (a latent dangling reference), or a write-through into sealed security policy.

11. Iterator invalidation

The dangerous shape is iterate a mutable container while it can be mutated during the walk. The mutation almost always arrives through a callback invoked inside the loop body that re-enters Hull and adds/removes/grows the very container being walked.

Safe patterns to require: snapshot array (copy the elements/handles before the loop), index-based iteration with explicit mutation rules (append-only + re-read count), two-phase update/delete queue (mark in phase 1, apply after the loop), or sealed immutable array (registries that are static const — confirm they're never mutated at runtime). Document, per container, whether mutation during iteration is allowed. Do not expose raw container storage if growth/realloc can occur.

12. Generational handles for long-lived objects

For long-lived runtime objects currently referenced by raw pointers across an API boundary (script → C), prefer a generational handle over a raw pointer so a stale reference fails validation instead of dereferencing freed memory:

typedef struct { uint32_t index; uint32_t generation; } HullHandle;

The backing table slot carries generation + an alive bit; lookup validates slot.generation == handle.generation and returns NULL on mismatch. Apply this ONLY where it improves safety without broad churn:

Today Hull's binding layer mostly uses the "null the userdata's opaque pointer on close/destroy, methods fail closed" pattern — an acceptable hand-rolled equivalent. Flag any binding where close/destroy frees the underlying struct but a suspended/async continuation can still resume against it (that's where the fail-closed null is insufficient).

13. Aliasing & single-owner mutation

Recommended naming discipline in comments/types: borrow / read-only view (caller must not free; valid only for the call), owned object (caller frees), arena-owned (freed by the arena's destroy, never individually), handle reference (validated each use, may be stale).

14. Data races & thread affinity

Hull's model: a single event-loop thread owns all core policy / security state; a worker thread pool runs db.async / compute.async / gpu.async jobs on deep-copied inputs; Keel returns each job's done_fn/cancel_fn to the event-loop thread.

Prefer, in order: single-threaded event loop for policy state → immutable sealed shared config → message passing (deep-copied inputs to workers) → explicit atomic queues only at the boundary. Document thread affinity for security-sensitive state. TSan CI (make tsan) must cover the worker-pool paths for each build flavor that ships a pool.

15. Debug-build defensive checks

Under make debug (and TSAN/MSAN where relevant), the trusted core should carry lightweight, compiled-out-in-release checks. When auditing new code in a security path, expect (or recommend) these:

• arena owner / lifetime asserts (alloc-after-destroy, alloc-after-seal both return NULL / assert — the bump arena already does)
• ASan poisoning of freed/reset arena regions (stale read traps)
• stale-handle detection (generation mismatch → NULL)
• mutation-during-iteration asserts (e.g. a dispatch_depth/iterating flag that aborts on re-entrant mutation)
• thread-affinity asserts for security-sensitive state (hl_assert_on_event_loop())

These are the runtime tripwires that turn a latent lifetime/race bug into a loud test failure under the sanitizer matrix.

Audit Procedure

When /c-audit is invoked:

1. Locate Files

   src/cap/*.c                 # Shared capability layer
   src/runtime/js/*.c          # QuickJS runtime integration
   src/runtime/lua/*.c         # Lua 5.4 runtime integration
   src/main.c                  # Entry point
   include/hull/*.h            # Public headers
   tests/test_*.c              # Test files
   Makefile                    # Build configuration
   

2. Scan for Critical Issues

   • Search for unsafe functions: strcpy, sprintf, gets, strcat
   • Search for unsafe integer parsing: atoi, atol, atof
   • Search for unchecked allocations: malloc/calloc without NULL check
   • Search for integer overflow in size calculations
   • Search for missing bounds checks on array access
   • Search for unchecked fwrite()/fread() return values
   • Search for direct SQLite/file I/O calls in runtime bindings (bypass of hl_cap_*)
   • Search for ROP/JOP defeats (§9.2):
     • mmap.*PROT_EXEC.*PROT_WRITE / mprotect.*PROT_EXEC.*PROT_WRITE (RWX)
     • dlopen / dlsym / dlmopen (runtime symbol resolution)
     • alloca( with data-derived size (stack-clash defeat)
     • new global function-pointer arrays missing static const
     • __attribute__((constructor)) (pre-main mutation)
     • __attribute__((no_stack_protector)) / ((naked)) / ((interrupt))
     • hardcoded executable address literals (0x[0-9a-f]{6,})
     • -rdynamic / -Wl,-export-dynamic in any Makefile change
   • Search for lifetime / aliasing / race hazards (§§10-14):
     • every realloc( → does any pointer/iterator held across it dangle? (§10.1)
     • free( in shared/ or utils/ or any security path (should be ~zero; §10)
     • callback invocations inside a loop over a mutable container (§11)
     • *_from_buffer / borrowed-view constructors returning a long-lived handle without pinning the source (§13)
     • public hl_* getters returning a non-const pointer into a buffer a later call can realloc/free (§10.1, §13)
     • worker work_fn (db/wasm/gpu async) reads/writes of a shared cache, global, or output stream without the owning mutex/lock (§14)
     • every entry point that calls pool_create → does it FIRST call hl_event_loop_mark_current() AND hl_log_make_threadsafe()? Check serve.c AND serve_cli.c (CLI flavor) (§14)

3. Review Public API

   • Check all public functions in headers (hl_* prefix)
   • Verify NULL checks on pointer parameters
   • Verify bounds checks on size parameters

4. Check Resource Management

   • Every _init()/_create() has matching _free()/_close()
   • Error paths free allocated resources
   • No memory leaks on early returns
   • SQLite statements finalized on error paths
   • QuickJS/Lua contexts cleaned up on error paths

5. Check Capability Boundaries

   • JS/Lua bindings only access resources through hl_cap_* functions
   • Path validation enforced before every filesystem operation
   • SQL always parameterized
   • Sandbox restrictions in place (no eval, no io/os libs)

6. Detect Dead Code

   • Compile with -Wunused flags
   • Find unused static functions
   • Find unused variables and parameters
   • Flag commented-out or #if 0 code blocks

7. Check Build Hardening (§9)

   • Probe-based flag set still applied (make hardening reports the expected list for this toolchain).
   • Verifier passes (make check-hardening, or scripts/check_hardening.sh build/hull directly).
   • No new C pattern from the §9.2 defeat list landed in this change (RWX, dlsym, unbounded alloca, naked attribute, etc.).
   • Sanitizer builds (make debug, make msan) still build and pass — if a new vendor TU was added, its *_CFLAGS block must inherit / re-add the sanitizer flags under ifdef DEBUG / ifdef MSAN.

8. Check Sealed Runtime Tables (§5b)

   • Any new boot-built mutable security policy flows through hl_seal_arena and is sealed before the resolver/sandbox runs.
   • Any new global function-pointer table (vtable, registry, callback array) is static const.
   • Any new xxd -i Makefile invocation uses XXD_CONST_SEAL or XXD_CONST_PIPE to land in .rodata.
   • Sealed-arena destroy stays LAST in every cleanup path.

9. Check Lifetime, Iteration & Concurrency (§§10-15)

   • Classify each allocation cluster by lifetime class (boot / module / request / scratch / third-party); flag scattered request-temporary malloc/free an arena would make leak-proof (§10).
   • One owner + one destroy per arena; no pointer retained past a reset (§10).
   • Every realloc re-derives held pointers; no getter returns a mutable internal pointer a later call can invalidate (§10.1).
   • No callback-driven loop mutates the container it walks; registries iterated are static const (§11).
   • Long-lived script-visible objects that app code can destroy while a reference (esp. a suspended/async continuation) lives: handle or fail-closed, never a bare dangling pointer (§12).
   • Sealed/policy state exposed const; single-owner mutation; borrowed buffers pinned by the long-lived objects that reference them (§13).
   • No off-event-loop mutation of policy/dispatch/manifest state; shared worker-touched caches under the owning mutex; every pool_create entry point marks the loop thread + makes log thread-safe first; audit/log stream writes from workers are lock-wrapped (§14).
   • For a large audit, fan out §10/§11/§14 to parallel subagents (one per dimension over cap/ + runtime/ + the worker files), then reconcile against known prior hardening before reporting.

10. Generate Report Format as markdown table with findings, severity, file:line, and suggested fix.

Report Format

## C Audit Report: Hull

**Date:** YYYY-MM-DD
**Files Scanned:** N
**Issues Found:** N (Critical: N, High: N, Medium: N, Low: N)

### Critical Issues

| # | File:Line | Issue | Current Code | Suggested Fix |
|---|-----------|-------|--------------|---------------|
| C1 | src/cap/hull_cap_db.c:42 | Buffer overflow | `strcpy(buf, src)` | `snprintf(buf, sizeof(buf), "%s", src)` |

### High Issues
...

### Medium Issues
...

### Low Issues
...

### Lifetime / ownership / concurrency summary (§§10-14)
For a lifetime/race-focused audit, ALSO report these dimensions explicitly
(state "CLEAN" where there is nothing — coverage matters):
- Risky allocation / `free` sites (by lifetime class).
- `realloc` / iterator-invalidation risks found.
- Raw-pointer lifetime / borrow-outlives-source risks found.
- Shared mutable state / data-race risks found (per build flavor).
- Changes made vs. cases intentionally left unchanged.
- Larger architectural changes recommended but NOT implemented.
- Remaining residual risks.

### Recommendations
1. ...

Fix Mode (--fix)

When --fix is specified:

1. Generate the audit report first
2. For each fixable issue, apply the transformation
3. Rebuild (make)
4. Re-run tests (make test)
5. Report any test failures or new warnings

Auto-fixable Issues:

• strcpy -> snprintf with buffer size
• sprintf -> snprintf with buffer size
• atoi -> strtol with validation
• Missing NULL checks (add early return)
• Missing malloc return check (add NULL check)
• Integer overflow in size calc -> calloc or overflow check
• Missing size_t casts in size calculations
• Unused local variables (remove)
• Unused static functions (remove)
• static T table[] for dispatch / vtable / registry → add const (§5b)
• New xxd -i Makefile rule missing const post-process → add XXD_CONST_SEAL / XXD_CONST_PIPE (§5b)
• Bounded alloca(SMALL_CONSTANT) left as-is; data-derived alloca(n) → flag, propose malloc + size check (§9.2, not auto-fix)

NOT Auto-fixable (require manual review):

• Logic errors
• Resource leaks in complex control flow
• Capability boundary violations
• Sandbox escape paths
• Architectural changes
• New W^X primitives (mmap/mprotect with PROT_WRITE|PROT_EXEC)
• New dlopen / dlsym introduced in C
• Sealed-arena destroy reordering — must trace every aliasing consumer's lifetime
• New __attribute__((constructor)) — needs design review of init order
• Probe regressions: if make hardening reports fewer flags than before on the same toolchain, the probe macro or surrounding ifdef was likely broken — manual investigation