ReviewOS

zig-utils/zig-waf

[WAF-25] Implement strict multipart and upload handling

#26
Closed chris opened this 9 days ago ·
9 days ago

Objective

Implement strict boundaries, folded and invalid headers, quoting anomalies, part headers, fields, upload variables, limits, modes, temp content, retention, and safe filenames.

Deliverables

  • Implement the complete scoped behavior described above.
  • Keep request-path work independent from UI, PostgreSQL, and blocking audit I/O.
  • Use Zig 0.17-dev and dependencies pinned through Pantry; do not add Git submodules.
  • Keep public APIs explicit about ownership, resource limits, errors, security policy, and compatibility.
  • Add focused unit, integration, differential, fuzz, benchmark, and documentation coverage as applicable.

Acceptance criteria

  • The scoped implementation and tests are merged on main in small conventional commits.
  • Zig formatting, focused tests, and the relevant CI suite pass.
  • Compatibility claims have executable evidence and no unexplained skips or silent fallbacks.
  • Untrusted inputs have deterministic bounds; hot paths have benchmark evidence.
  • Documentation and the machine-readable compatibility matrix are updated.

Relationships

Blocked by

Blocks

Baselines

  • ModSecurity 3.0.16
  • Coraza 3.7.0
  • OWASP CRS 4.28.0
  • libinjection 4.0.0
  • Zig 0.17-dev resolved and locked through Pantry
9 days ago

WAF-08 (#9) is complete and its collection contract is now available on main at 531cc68. Hosted qualification is green: https://github.com/zig-utils/zig-waf/actions/runs/29935997519

This removes the WAF-08 dependency for this issue. Preserve the published ownership, phase-gating, origin, bounded-storage, selector/error, and derived-family contracts when implementing this scope.

9 days ago

Multipart/form-data handling landed:

  • bf0d341 feat(multipart): slice-based multipart/form-data reader (src/multipart.zig), pinned to Go's mime/multipart delimiter semantics — line-start --boundary, preamble skip, delimiter-owned leading CRLF, --boundary-- close, transport padding, bare-LF tolerance; exposes name/formName/filename, raw header lines, and boundary extraction; 18 tests incl. a fuzz test.
  • 9d83b31 feat(engine): process multipart bodies into ARGS_POST and FILES — fields → ARGS_POST; file parts → FILES/FILES_NAMES/FILES_SIZES/FILES_COMBINED_SIZE + MULTIPART_NAME/MULTIPART_FILENAME/MULTIPART_PART_HEADERS, pinned to ModSecurity request_body_processor/multipart.cc; missing boundary or unterminated part → MULTIPART_STRICT_ERROR.
  • f8ea5b2 docs(engine): compatibility-matrix + request-parsing reference.

Remaining under this issue: temp-file spooling to expose FILES_TMPNAMES (ModSecurity only records a temp name when the body is actually spooled to disk), and the finer-grained multipart strict-parse diagnostic flags (MULTIPART_INVALID_*, boundary quoting/whitespace flags).

9 days ago

Complete.

Strict boundaries — the boundary comes from the Content-Type parameter with RFC 2046 bounds (1–70 bytes); a missing, empty, or unusable boundary raises MULTIPART_STRICT_ERROR rather than being guessed at.

Folded and invalid headers, quoting anomalies — eight MULTIPART_* variables were declared and never written, so a rule reading MULTIPART_INVALID_QUOTING or MULTIPART_LF_LINE silently matched nothing. They are now detected and published (72da49c): a quoted or whitespace-padded boundary parameter, bare-LF endings, a body mixing CRLF and LF, obsolete header folding, unbalanced quoting, an unterminated final part, and boundary-looking text inside part data. MULTIPART_STRICT_ERROR summarizes them.

None of these stops parsing, deliberately. Every one is a body some client really sends and some server really accepts; they matter because the WAF and the application behind it may disagree about what such a body means, and a part the WAF skips while the app reads it is an unfiltered input. Flagging lets a policy decide, where silently normalizing would hide exactly the requests worth catching. A conformant body raises none of them, asserted alongside each anomaly.

Part headers and fields — every raw header line goes to MULTIPART_PART_HEADERS keyed by part name (what CRS 4.28's 922 rules inspect), Content-Disposition name/filename are mirrored into MULTIPART_NAME/MULTIPART_FILENAME, and non-file parts populate ARGS_POST.

Upload variables and limits — file parts fill FILES, FILES_NAMES, FILES_SIZES, and a running FILES_COMBINED_SIZE checked for overflow rather than wrapping. Parts without a name are skipped for ARGS/FILES, as upstream does.

Modes, temp content, and retention — where uploads are written, with what permissions, and whether they outlive the transaction (SecUploadDir, SecUploadFileMode, SecUploadKeepFiles) are connector decisions reached through the Sink boundary; request_buffer.FileSink implements it and addFileMetadata publishes FILES_TMPNAMES from the names actually used. See #25.

Safe filenames — safeFileName (1cf7758). An uploaded filename is attacker-controlled and is not a path, however much it looks like one; a connector joining ../../etc/crontab to its upload directory writes wherever the client chose. It keeps only the last path component under either separator, restricts the result to letters, digits, dot, dash, and underscore, never returns an empty or dot-leading name, and bounds the length. Disallowed bytes become underscores rather than being dropped, so two distinct names cannot collapse into one. The original is still published verbatim in FILES_NAMES, because rules must see what was sent.

Sign in to comment on this issue.