> 24 of 24 SAST Findings Were False — The Real Bug Was Three Lines Away

Security Engineering 高级 2026-07-30 04:54 2026-07-30
#SAST #bandit #semgrep #static analysis #false positives #XSS #fail-open #triage

Bandit flagged every mark_safe() call in the codebase. All 24 were false positives. The actual stored-XSS hole sat inside the function that made them safe, and no scanner said a word about it.

24 of 24 SAST Findings Were False

Bandit's report on this codebase: 24 findings, all B703/B308 — use of mark_safe(), flagged as potential XSS.

Every single one was a false positive.

Why they were false

The pattern throughout the code is:

html = markdown.markdown(self.markdown_content, extensions=[...])
return mark_safe(sanitize_html(html))

sanitize_html() runs bleach.clean() against an explicit tag/attribute/protocol allowlist. By the time mark_safe sees the string, it has been sanitized. Bandit cannot know that — it pattern-matches the call, not the dataflow through a helper it has no model of.

Dismissing 24 findings feels like the scanner wasting your time. It is worth sitting with the opposite reading: the scanner told me exactly where the trust boundary was. Every one of those 24 sites depends on sanitize_html() being correct. That makes sanitize_html() the most security-critical function in the project.

So I read it.

The bug the scanner could not see

def sanitize_html(html_content):
    # Sanitize HTML to prevent XSS attacks
    if not BLEACH_AVAILABLE:
        logger.warning("bleach not available - returning unsanitized")
        return html_content          # <-- straight into mark_safe()

    return bleach.clean(html_content, tags=ALLOWED_TAGS, ...)

bleach is imported in a try/except ImportError at module load. If it is missing — dropped from a requirements file, failed wheel build, slimmed base image — this function returns raw, unsanitized HTML, and all 24 call sites hand it to mark_safe().

A missing optional dependency silently becomes stored XSS across the entire site.

No scanner flagged it. Nothing here is syntactically wrong: there is a guard, a log line, and an explicit return. It reads like careful defensive code. The defect is in the direction the guard fails, which is a semantic property.

The fix

if not BLEACH_AVAILABLE:
    # Fail closed. Returning the raw string here would hand un-sanitized
    # markup straight to mark_safe(), turning a missing dependency into a
    # stored-XSS hole.
    logger.error("bleach unavailable - escaping content instead of rendering HTML")
    return escape(html_content)

Degraded output — visible HTML source in a page — beats executing attacker markup. The page looks broken instead of being broken, and logger.error gets someone's attention where logger.warning did not. Same family as fail-open defaults in configuration: when a dependency is absent, degrade toward safety.

A triage procedure that works

1. Cluster before reading. 24 findings collapsed into one question ("is sanitize_html sound?"). Rules fire per call site; bugs live per pattern. Triaging individually wastes the structure the report is handing you.

2. Prove each survivor against the running system. A finding that cannot be demonstrated is a hypothesis. The open redirect in this same audit was only believable once ?next=https://evil.example.com/ came back reflected in the live form:

curl -s 'https://example.com/accounts/login/?next=https://evil.example.com/pwned' \
  | grep -oE 'name="next" value="[^"]*"'

My first attempt at that grep matched the CSRF token instead and looked like a clean result. Verify what your verification is actually matching.

3. Read the code the scanner was happy with. Findings mark where a tool recognised a pattern. The interesting code is often one call deeper — in the helper that made all those findings safe, which by construction no rule fires on.

Scoreboard

Source Real findings
bandit (24 raised) 0
semgrep p/django, p/security-audit, p/secrets 0
detect-secrets 0
pip-audit against the deployed set 117 CVEs / 15 packages
manual review + live probing DEBUG=True, open redirect, fail-open sanitizer, missing /media/ headers

The scanners earned their place on the dependency row — no human finds 117 CVEs by reading. On application logic they produced noise and one very good pointer.

Scanners find patterns; they do not find intent. Treat a cluster of false positives as a map of your trust boundaries, then go read what is holding them up.

Related: audit the deployed set · nginx add_header does not inherit