diff --git a/apps/interop_server_manual.zig b/apps/interop_server_manual.zig index 2cba909..cdbe66d 100644 --- a/apps/interop_server_manual.zig +++ b/apps/interop_server_manual.zig @@ -349,10 +349,10 @@ fn pollH3Server(h3c: *h3.H3Connection, alloc: std.mem.Allocator, www_dir: []cons h3c.sendResponse(hdr.stream_id, &resp_headers, file_data) catch {}; }, .data => { - // Drain body to clear pending state - var sink: [4096]u8 = undefined; - while (h3c.recvBody(&sink) > 0) {} - }, + // Drain body to clear pending state + var sink: [4096]u8 = undefined; + while (h3c.recvBody(&sink) > 0) {} + }, .settings, .finished, .goaway, .connect_request, .shutdown_complete, .request_cancelled => {}, } } @@ -376,24 +376,11 @@ fn pollH0Server(h0c: *h0.H0Connection, www_dir: []const u8) void { } fn readFileFromWww(alloc: std.mem.Allocator, www_dir: []const u8, path: []const u8) ![]u8 { - var clean_path = path; - while (clean_path.len > 0 and clean_path[0] == '/') { - clean_path = clean_path[1..]; - } - if (clean_path.len == 0) clean_path = "index.html"; - + var clean_path_buf: [4096]u8 = undefined; var full_path_buf: [4096]u8 = undefined; - var pos: usize = 0; - @memcpy(full_path_buf[pos..][0..www_dir.len], www_dir); - pos += www_dir.len; - if (www_dir.len > 0 and www_dir[www_dir.len - 1] != '/') { - full_path_buf[pos] = '/'; - pos += 1; - } - @memcpy(full_path_buf[pos..][0..clean_path.len], clean_path); - pos += clean_path.len; + const full_path = try h0.buildSafeFilePath(www_dir, path, &clean_path_buf, &full_path_buf); - return std.fs.cwd().readFileAlloc(alloc, full_path_buf[0..pos], 10 * 1024 * 1024); + return std.fs.cwd().readFileAlloc(alloc, full_path, 10 * 1024 * 1024); } fn loadFile(alloc: std.mem.Allocator, path: []const u8) ![]u8 { diff --git a/src/h0/connection.zig b/src/h0/connection.zig index 8f3aba9..1d17e64 100644 --- a/src/h0/connection.zig +++ b/src/h0/connection.zig @@ -20,6 +20,82 @@ const MAX_REQUEST_LINE = 4096; /// Maximum file size to serve (10MB). const MAX_FILE_SIZE = 10 * 1024 * 1024; +const REQUEST_REJECTED_ERROR: u64 = 1; + +fn copyDefaultPath(out_buf: []u8) ![]const u8 { + const default_path = "index.html"; + if (out_buf.len < default_path.len) return error.PathTooLong; + @memcpy(out_buf[0..default_path.len], default_path); + return out_buf[0..default_path.len]; +} + +/// Convert an untrusted request path into a normalized relative filesystem path. +/// Rejects path traversal, backslash separators, NUL bytes, and oversized paths. +pub fn sanitizeRelativePath(path: []const u8, out_buf: []u8) ![]const u8 { + var trimmed = path; + if (mem.indexOfAny(u8, trimmed, "?#")) |idx| { + trimmed = trimmed[0..idx]; + } + + while (trimmed.len > 0 and trimmed[0] == '/') { + trimmed = trimmed[1..]; + } + + if (trimmed.len == 0) { + return copyDefaultPath(out_buf); + } + + var out_len: usize = 0; + var segments = mem.splitScalar(u8, trimmed, '/'); + while (segments.next()) |segment| { + if (segment.len == 0 or mem.eql(u8, segment, ".")) continue; + if (mem.eql(u8, segment, "..")) return error.PathTraversal; + if (mem.indexOfScalar(u8, segment, 0) != null) return error.InvalidPath; + if (mem.indexOfScalar(u8, segment, '\\') != null) return error.InvalidPath; + + const separator_len: usize = if (out_len == 0) 0 else 1; + if (out_len + separator_len + segment.len > out_buf.len) return error.PathTooLong; + if (separator_len == 1) { + out_buf[out_len] = '/'; + out_len += 1; + } + @memcpy(out_buf[out_len..][0..segment.len], segment); + out_len += segment.len; + } + + if (out_len == 0) { + return copyDefaultPath(out_buf); + } + + return out_buf[0..out_len]; +} + +pub fn buildSafeFilePath(root_dir: []const u8, request_path: []const u8, clean_path_buf: []u8, full_path_buf: []u8) ![]const u8 { + const clean_path = try sanitizeRelativePath(request_path, clean_path_buf); + + if (root_dir.len == 0) { + return std.fmt.bufPrint(full_path_buf, "{s}", .{clean_path}) catch error.PathTooLong; + } + if (root_dir[root_dir.len - 1] == '/') { + return std.fmt.bufPrint(full_path_buf, "{s}{s}", .{ root_dir, clean_path }) catch error.PathTooLong; + } + return std.fmt.bufPrint(full_path_buf, "{s}/{s}", .{ root_dir, clean_path }) catch error.PathTooLong; +} + +fn copyRequestPath(line: []const u8, out_buf: []u8) !?[]const u8 { + if (!mem.startsWith(u8, line, "GET ")) return null; + + const path = line[4..]; + if (path.len > out_buf.len) return error.PathTooLong; + + @memcpy(out_buf[0..path.len], path); + return out_buf[0..path.len]; +} + +fn rejectRequestStream(stream: *stream_mod.Stream) void { + stream.send.reset(REQUEST_REJECTED_ERROR); + stream.recv.stopSending(REQUEST_REJECTED_ERROR); +} /// Event returned by poll(). pub const H0Event = union(enum) { @@ -108,27 +184,15 @@ pub const H0Connection = struct { /// Serve a file from the given root directory on the specified stream. pub fn serveFile(self: *H0Connection, stream_id: u64, root_dir: []const u8, path: []const u8) !void { - // Sanitize path: strip leading "/" - var clean_path = path; - while (clean_path.len > 0 and clean_path[0] == '/') { - clean_path = clean_path[1..]; - } - if (clean_path.len == 0) clean_path = "index.html"; - - // Build full filesystem path + var clean_path_buf: [MAX_REQUEST_LINE]u8 = undefined; var full_path_buf: [4096]u8 = undefined; - var full_path_pos: usize = 0; - @memcpy(full_path_buf[full_path_pos..][0..root_dir.len], root_dir); - full_path_pos += root_dir.len; - if (root_dir.len > 0 and root_dir[root_dir.len - 1] != '/') { - full_path_buf[full_path_pos] = '/'; - full_path_pos += 1; - } - @memcpy(full_path_buf[full_path_pos..][0..clean_path.len], clean_path); - full_path_pos += clean_path.len; - full_path_buf[full_path_pos] = 0; - - const full_path = full_path_buf[0..full_path_pos]; + const full_path = buildSafeFilePath(root_dir, path, &clean_path_buf, &full_path_buf) catch |err| { + std.log.warn("H0: rejected request path '{s}': {any}", .{ path, err }); + const streams_map = &self.quic_conn.streams; + const stream = streams_map.getStream(stream_id) orelse return; + rejectRequestStream(stream); + return; + }; // Read file const file_data = std.fs.cwd().readFileAlloc(self.allocator, full_path, MAX_FILE_SIZE) catch |err| { @@ -162,6 +226,10 @@ pub const H0Connection = struct { const data = stream.recv.read() orelse { // No data available - check if stream is finished if (stream.recv.finished) { + if (self.stream_bufs.getPtr(stream_id)) |buf| { + buf.deinit(self.allocator); + _ = self.stream_bufs.remove(stream_id); + } self.finished_streams.put(stream_id, {}) catch {}; return H0Event{ .finished = stream_id }; } @@ -174,6 +242,14 @@ pub const H0Connection = struct { if (!buf_entry.found_existing) { buf_entry.value_ptr.* = .{ .items = &.{}, .capacity = 0 }; } + if (buf_entry.value_ptr.items.len +| data.len > MAX_REQUEST_LINE) { + std.log.warn("H0: rejecting oversized request line on stream {d}", .{stream_id}); + buf_entry.value_ptr.deinit(self.allocator); + _ = self.stream_bufs.remove(stream_id); + self.finished_streams.put(stream_id, {}) catch {}; + rejectRequestStream(stream); + continue; + } try buf_entry.value_ptr.appendSlice(self.allocator, data); // Check for complete request line @@ -181,10 +257,10 @@ pub const H0Connection = struct { if (mem.indexOf(u8, buf_data, "\r\n")) |idx| { // Parse "GET /path" const line = buf_data[0..idx]; - if (mem.startsWith(u8, line, "GET ")) { - const path = line[4..]; - @memcpy(self.path_buf[0..path.len], path); + 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 = .{ @@ -205,3 +281,56 @@ pub const H0Connection = struct { return null; } }; + +test "sanitizeRelativePath normalizes safe request paths" { + var buf: [MAX_REQUEST_LINE]u8 = undefined; + + try std.testing.expectEqualStrings("index.html", try sanitizeRelativePath("/", &buf)); + try std.testing.expectEqualStrings("nested/file.txt", try sanitizeRelativePath("//nested/./file.txt?download=1#frag", &buf)); + try std.testing.expectEqualStrings("foo/bar", try sanitizeRelativePath("/foo//bar/", &buf)); +} + +test "sanitizeRelativePath rejects traversal and invalid separators" { + var buf: [MAX_REQUEST_LINE]u8 = undefined; + + try std.testing.expectError(error.PathTraversal, sanitizeRelativePath("/../etc/passwd", &buf)); + try std.testing.expectError(error.PathTraversal, sanitizeRelativePath("/safe/../../etc/passwd", &buf)); + try std.testing.expectError(error.InvalidPath, sanitizeRelativePath("/foo\\bar", &buf)); +} + +test "buildSafeFilePath bounds the final path" { + var clean_path_buf: [MAX_REQUEST_LINE]u8 = undefined; + var full_path_buf: [32]u8 = undefined; + var long_request: [MAX_REQUEST_LINE]u8 = undefined; + + @memset(&long_request, 'a'); + + try std.testing.expectError( + error.PathTooLong, + buildSafeFilePath("/www", long_request[0..], &clean_path_buf, &full_path_buf), + ); +} + +test "copyRequestPath extracts GET paths" { + var buf: [16]u8 = undefined; + + const path = (try copyRequestPath("GET /file.txt", &buf)).?; + try std.testing.expectEqualStrings("/file.txt", path); + try std.testing.expect((try copyRequestPath("POST /file.txt", &buf)) == null); +} + +test "copyRequestPath rejects oversized paths before memcpy" { + var buf: [4]u8 = undefined; + + try std.testing.expectError(error.PathTooLong, copyRequestPath("GET /abcd", &buf)); +} + +test "rejectRequestStream cancels both directions" { + var stream = stream_mod.Stream.init(std.testing.allocator, 0); + defer stream.deinit(); + + rejectRequestStream(&stream); + + try std.testing.expectEqual(@as(?u64, REQUEST_REJECTED_ERROR), stream.send.reset_err); + try std.testing.expectEqual(@as(?u64, REQUEST_REJECTED_ERROR), stream.recv.stop_sending_err); +}