Conversation
Reject traversal segments and invalid separators before joining request paths under the interop www root. This also bounds the final path build to avoid fixed-buffer overflows when handling oversized request paths. Co-authored-by: Codex <noreply@openai.com>
Drop the opportunistic change to the sibling interop server so this branch stays focused on the manual interop path traversal and shared H0 file-serving path handling only. Co-authored-by: Codex <noreply@openai.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens the interop file-serving logic by introducing a shared request-path sanitization + safe join helper, then migrating the HTTP/0.9 and HTTP/3 interop servers to use it to prevent traversal and path-length buffer issues.
Changes:
- Add
sanitizeRelativePathandbuildSafeFilePathhelpers to normalize/validate untrusted request paths and safely join them under a configured root. - Update HTTP/0.9 serving (
H0Connection.serveFile) to use the shared safe-path builder and reject invalid paths. - Update both the manual and event-loop interop HTTP/3 file readers to use the same safe-path builder (removing duplicated unsafe path-join code).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/h0/connection.zig |
Adds path sanitization + safe join helpers, switches H0 file serving to use them, and adds unit tests. |
apps/interop_server_manual.zig |
Replaces ad-hoc path stripping + unchecked buffer copies with h0.buildSafeFilePath. |
apps/interop_server.zig |
Uses h0.buildSafeFilePath for HTTP/3 file reads so the main interop server isn’t left on the old vulnerable join logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Reject oversized H0 request lines before they are appended or copied into the fixed path buffer, and free per-stream request buffers once a stream finishes or a request is emitted. Co-authored-by: Codex <noreply@openai.com>
There was a problem hiding this comment.
Pull request overview
This PR hardens the interop file-serving logic against path traversal and oversized-path memory hazards by introducing shared, bounded path sanitization/joining and applying it to both HTTP/0.9 and the manual HTTP/3 server code paths.
Changes:
- Add
sanitizeRelativePath+buildSafeFilePathhelpers to normalize/unify file-serving path handling and reject traversal/invalid/oversized paths. - Replace unchecked stack-buffer
@memcpypath construction in HTTP/0.9 serving and the manual interop server with boundedbufPrint-based joining. - Add regression tests covering normalization, traversal rejection, and path-length bounds; and tighten HTTP/0.9 request-line buffering to cap at
MAX_REQUEST_LINEwith cleanup.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/h0/connection.zig | Introduces shared path sanitization + safe join helpers, applies them to file serving, adds request-line size bounding/cleanup, and adds tests. |
| apps/interop_server_manual.zig | Switches manual H3/H0 interop file reading to use the shared safe path join helper (and fixes indentation in an event arm). |
Comments suppressed due to low confidence (1)
src/h0/connection.zig:263
- After successfully parsing a request line, the stream is added to
finished_streams, so future polls will stop reading from it. If a client sends additional bytes after the request line (maliciously or accidentally), they can still be buffered by QUIC flow control even though the app won’t drain them. Consider callingstream.recv.stopSending(<app error code>)after extracting the request (or draining until FIN) before marking the stream finished.
if (try copyRequestPath(line, &self.path_buf)) |path| {
self.path_len = path.len;
buf_entry.value_ptr.deinit(self.allocator);
_ = self.stream_bufs.remove(stream_id);
// Mark as finished so subsequent polls skip this stream
self.finished_streams.put(stream_id, {}) catch {};
return H0Event{ .request = .{
.stream_id = stream_id,
.path = self.path_buf[0..self.path_len],
} };
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Use RESET_STREAM plus STOP_SENDING for invalid path and oversized request-line rejection, and add a focused unit test for the shared rejection helper. Co-authored-by: Codex <noreply@openai.com>
Summary
.., backslash separators, NUL bytes, and oversized request paths before mapping them under the interopwwwrootVulnerability
Before this change, the manual interop server and shared HTTP/0.9 file-serving path handling only stripped leading
/characters and then copied the remaining request path into a fixed 4096-byte stack buffer with unchecked@memcpyoperations.That made two network-reachable cases unsafe:
GET /../etc/passwd\r\non HQ/H0, or:path = /../../proc/self/environon H3, could escape the intendedwwwroot and read arbitrary files/followed by 5000abytes, could overrun the stack buffer and panic in safe builds or corrupt memory in unchecked buildsThe new helper performs lexical validation up front, removes redundant separators and
.segments, strips query/fragment suffixes, defaults/toindex.html, and uses bounded formatting when constructing the final filesystem path.Validation
zig build testzig buildzig build fuzzReferences
realpathand instead validates lexically before joining): remove realpath() from the standard library ziglang/zig#19353