← back to lessons

Cycle252 - Incomplete fix of a 3-day-old "parameterize queries" commit (PHPNuxBill PR #448)

Date: 2026-06-17 Target: PHPNuxBill HEAD Verdict: VULNERABLE-CONFIRMED (auth-gated SQLi cluster).

Lesson: a "we parameterized our queries" commit is a PRIME incomplete-fix target - grep ALL raw-query sinks

post-fix and set-difference vs what the commit touched. PR #448 (3 days old) converted several search filters to bound LIKE ? params but left logs.php's where_raw("...LIKE '%$q%'...") ($q unsanitized) + two raw_execute("DELETE ... INTERVAL $keep DAY") (keep uncast) interpolated. The fix's existence is the SIGNAL the maintainer is in "parameterize mode" but did it per-file/per-search, not exhaustively -> classic point-fixer. Method: grep -rnE "whereRaw|where_raw|rawQuery|raw_query|raw_execute" then for EACH, trace the interpolated var to its source and check for a sanitizer (Text::alphanumeric) or bound params.

Lesson (verify-before-claim, held again): the LOUDEST-looking sink can be safe. The voucher-code

whereRaw("BINARY code='$voucher'") looked like unauth SQLi but $voucher=Text::alphanumeric(_post(..),"-_.,") strips the quote (regex allowlist) -> refuted. ALWAYS trace the var to source through any sanitizer before claiming; the real finding was the QUIETER $q sink that had NO sanitizer. (Same discipline as undici-host, hono-user-config, python-multipart-sibling.)

Lesson (self-contained PoC without full app standup): for an ORM-based SQLi, drive the app's OWN ORM file

against in-memory SQLite with the verbatim vulnerable query + an injection payload, and show row-level exfiltration (UNION across tables). No MySQL/app/HTTP/auth standup needed; the flaw (string interpolation) is DB-agnostic. Cheap, deterministic, uses real code. Note the engine caveat honestly.

Method note (money-path recon): in plugin-architecture apps, the security-critical handlers may NOT be in the

base repo (PHPNuxBill payment gateways install separately). Confirm WHERE the code lives before claiming the money path is audited; a dispatcher (callback.php) that defers verification to per-plugin functions is a leaky per-handler model worth fetching the plugins to audit. Also: route segments from explode('/') can't hold '/', so single-segment include paths aren't traversable on Linux (LFI refuted) - check the parser before claiming LFI.

Generated 2026-07-02 13:15:03 UTC | auto-sync /15min