Eighteen false positives, one missed entry point and a key that survived redaction: what real projects changed in PandaWard

Running PandaWard on real repositories and under a fuzzer found a secret that redaction put back together, a retest that marked the wrong finding as fixed, eighteen false positives from four rules, and 213 server actions the inventory never listed. Four releases on 29 October fixed all but the last.

Category
Lessons learned
Published
Reading time
6 min read

Why this matters for a tool like PandaWard

PandaWard makes a few promises that are easy to write and hard to keep: a secret never appears in any output, a finding is remediated only when its check runs again and stops seeing it, and a control that could not be checked is never reported as clean. Its own tests and a corpus I wrote check those promises against cases I thought of. They cannot show what happens on code I did not write the rules for.

So after 0.2.3 I ran PandaWard on real repositories of my own and let the CI fuzzers keep running. Every problem below was filed as an issue before it was fixed, and each fix came with a test that fails on the old code.

A secret that redaction put back together

What was found. The CI fuzz target FuzzMobSFScorecard found an input for which the MobSF API key ended up in the saved raw output (#144). When a MobSF response is not valid JSON, PandaWard removed the key from the raw text and then ran line redaction. If the response held the key split by a control character, such as 0123456789\x01abcdef..., the removal did not match. Line redaction then sanitized the text, removed the control character and joined the key back together.

Why it matters. The guarantee is that a secret never appears in any output, not that it usually does not. A real MobSF server is unlikely to send that response, but the order of two steps decided whether the guarantee held, and nothing except the fuzzer would have noticed.

The fix. #145 sanitizes first, removes the key, runs line redaction, and removes the key once more. The failing input is now a fuzz seed, so the case runs on every build. The JSON path was never affected, because json.Marshal escapes control characters and the key stays split.

A fix that marked the wrong finding as fixed

What was found. A finding's identity was its rule, file, key and code snippet plus an occurrence counter (#149). With two identical hits in one file, for example two db.query("..." + id) lines, fixing the first one left a single hit, which took occurrence 1. The finding that was fixed stayed open and now pointed at the other line, and the one that was not fixed was marked remediated. Moving a file without changing it reported every finding in it as remediated plus new, and lost any triage on them.

Why it matters. "Remediated only when the check runs again" is the core of PandaWard's retest. If the ledger credits the wrong line, a person closes a ticket for code that is still vulnerable, and a false-positive decision disappears when someone renames a directory.

The fix. #150 aligns identical hits with existing findings by line, choosing the matching with the smallest total shift, instead of by occurrence order. A finding whose file no longer exists continues under a new path only when its own fingerprint proves the same key and snippet, or, for a collector that reports no snippet, when the new file has the exact SHA-256 the old file had. It keeps its ID, status and triage, and the ledger records a moved event. Existing fingerprints are computed the same way, so no ledger is renumbered by the upgrade. The pull request states the remaining limit: a file that is moved and edited, with a collector that reports no snippet, still reads as remediated plus new.

Eighteen false positives from four rules

Running PandaWard on my own Next.js and TypeScript projects produced findings that I reviewed and closed as false positives. Four rules accounted for eighteen of them:

  • #156: gitleaks.generic-api-key (E-005), 8 findings. Per-build preview and server action keys in .next/, which is gitignored and excluded by default.
  • #157: pandaward-js-ssrf (D-006), 3 findings. fetch to a fixed https://api.github.com origin, with path segments from stored data.
  • #158: code.webhook-no-signature (G-001, high), 1 finding. An OAuth redirect callback that checks state and uses PKCE, not a webhook.
  • #160: pandaward-js-sql-string (D-003, high), 6 findings. SQL text built from module constants and numbers, with no request data.

Why it matters. A finding costs a reviewer's time whether it is real or not. When most findings in a report are noise, people stop reading the report, and the real issue in it gets skipped with the rest. For a tool whose point is evidence, a high-severity finding has to mean something.

The fixes, all in v0.2.7. Each one narrows a rule only where the code shows the input is not controlled by a request, and each comes with test cases showing that the dangerous shape next to it is still reported:

  • Excludes for every adapter (#161). gitleaks walks the whole tree, so the default exclude of .next only reached PandaWard's built-in checks. The engine now drops signals in excluded paths after every adapter and writes how many it dropped into the run detail, so nothing disappears silently.
  • Fixed origins are not SSRF (#166). A URL that starts with a literal https://host/ or with an UPPER_CASE constant followed by / is a sanitizer. The / is required: ${GITHUB_API}${req.query.path} could become api.github.com.evil.example, and it is still reported.
  • Sign-in callbacks are not webhooks (#162). A callback route whose path names a sign-in flow (OAuth, OIDC, SAML, SSO and similar) is no longer a webhook, unless the path also says webhook, /hooks/, /notify or /ipn. /payment/callback is still a webhook.
  • SQL from constants (#165). Constants, lists built from them, numeric coercions and numbers are not reported on their own, but each interpolation is judged separately, so ${INVOICES_TABLE} WHERE id = ${req.query.id} still matches.

A missed entry point

What was found. In a Next.js App Router project every function behind a "use server" directive is a public POST endpoint. PandaWard's inventory listed only route.ts handlers (#163). In one project that was 213 "use server" occurrences across 79 files, 15 of them file-level modules, and none appeared in the entry point list.

Why it matters. This is the opposite of a false positive, and it is worse. The authorization reviews (C-001 to C-003) and the links between identity and authorization only look at listed entry points. A server action without an access check is exactly what they are there to find, and they never saw one. A false positive costs time; a false negative gives a clean-looking answer that is not true.

Status. #169 lists server actions as a new entry point kind, server_action, with file and line, for both module-level and inline directives, and counts them toward A-002. It is open and not part of a release yet.

Takeaways

  • Run the tool on code you did not write the rules for. Four rules that passed their own fixtures produced eighteen false positives on real projects.
  • Fuzz the redaction path, not only the parsers. The order of two correct steps was enough to leak a key.
  • Identity is part of correctness. A retest that credits the wrong line is a wrong answer, even if the counts look right.
  • When narrowing a rule, add the dangerous neighbor as a test case in the same change.
  • Treat a missing entry point as seriously as a wrong finding. Coverage gaps do not show up in a report unless the tool counts what it did not see.

References

Code and history behind this post

More

Theme