From 9727931fda50ae412c47bfd40ad1d1dcd06aada0 Mon Sep 17 00:00:00 2001 From: Techatrix <19954306+Techatrix@users.noreply.github.com> Date: Sun, 25 Feb 2024 12:07:12 +0100 Subject: [PATCH 1/2] fix integer overflow in indexOfPosLinear when needle.len > haystack.len --- lib/std/mem.zig | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/lib/std/mem.zig b/lib/std/mem.zig index f263b3e851..fc0c226894 100644 --- a/lib/std/mem.zig +++ b/lib/std/mem.zig @@ -1346,6 +1346,7 @@ pub fn lastIndexOfLinear(comptime T: type, haystack: []const T, needle: []const /// Consider using `indexOfPos` instead of this, which will automatically use a /// more sophisticated algorithm on larger inputs. pub fn indexOfPosLinear(comptime T: type, haystack: []const T, start_index: usize, needle: []const T) ?usize { + if (needle.len > haystack.len) return null; var i: usize = start_index; const end = haystack.len - needle.len; while (i <= end) : (i += 1) { @@ -1354,6 +1355,26 @@ pub fn indexOfPosLinear(comptime T: type, haystack: []const T, start_index: usiz return null; } +test indexOfPosLinear { + try testing.expectEqual(0, indexOfPosLinear(u8, "", 0, "")); + try testing.expectEqual(0, indexOfPosLinear(u8, "123", 0, "")); + + try testing.expectEqual(null, indexOfPosLinear(u8, "", 0, "1")); + try testing.expectEqual(0, indexOfPosLinear(u8, "1", 0, "1")); + try testing.expectEqual(null, indexOfPosLinear(u8, "2", 0, "1")); + try testing.expectEqual(1, indexOfPosLinear(u8, "21", 0, "1")); + try testing.expectEqual(null, indexOfPosLinear(u8, "222", 0, "1")); + + try testing.expectEqual(null, indexOfPosLinear(u8, "", 0, "12")); + try testing.expectEqual(null, indexOfPosLinear(u8, "1", 0, "12")); + try testing.expectEqual(null, indexOfPosLinear(u8, "2", 0, "12")); + try testing.expectEqual(0, indexOfPosLinear(u8, "12", 0, "12")); + try testing.expectEqual(null, indexOfPosLinear(u8, "21", 0, "12")); + try testing.expectEqual(1, indexOfPosLinear(u8, "212", 0, "12")); + try testing.expectEqual(0, indexOfPosLinear(u8, "122", 0, "12")); + try testing.expectEqual(1, indexOfPosLinear(u8, "212112", 0, "12")); +} + fn boyerMooreHorspoolPreprocessReverse(pattern: []const u8, table: *[256]usize) void { for (table) |*c| { c.* = pattern.len; From a07218cc431701d13f169f4896a440d87a5a47c1 Mon Sep 17 00:00:00 2001 From: Techatrix <19954306+Techatrix@users.noreply.github.com> Date: Sun, 25 Feb 2024 12:01:21 +0100 Subject: [PATCH 2/2] http: handle header fields with empty value --- lib/std/http/Client.zig | 6 ++-- lib/std/http/HeaderIterator.zig | 16 ++++++--- lib/std/http/Server.zig | 28 +++++++++------ lib/std/http/test.zig | 61 ++++++++++++++++++++++++++++++++- 4 files changed, 92 insertions(+), 19 deletions(-) diff --git a/lib/std/http/Client.zig b/lib/std/http/Client.zig index 5f580bd53e..95a84ceaa2 100644 --- a/lib/std/http/Client.zig +++ b/lib/std/http/Client.zig @@ -488,7 +488,7 @@ pub const Response = struct { var line_it = mem.splitSequence(u8, line, ": "); const header_name = line_it.next().?; const header_value = line_it.rest(); - if (header_value.len == 0) return error.HttpHeadersInvalid; + if (header_name.len == 0) return error.HttpHeadersInvalid; if (std.ascii.eqlIgnoreCase(header_name, "connection")) { res.keep_alive = !std.ascii.eqlIgnoreCase(header_value, "close"); @@ -774,7 +774,7 @@ pub const Request = struct { } for (req.extra_headers) |header| { - assert(header.value.len != 0); + assert(header.name.len != 0); try w.writeAll(header.name); try w.writeAll(": "); @@ -1515,11 +1515,13 @@ pub fn open( ) RequestError!Request { if (std.debug.runtime_safety) { for (options.extra_headers) |header| { + assert(header.name.len != 0); assert(std.mem.indexOfScalar(u8, header.name, ':') == null); assert(std.mem.indexOfPosLinear(u8, header.name, 0, "\r\n") == null); assert(std.mem.indexOfPosLinear(u8, header.value, 0, "\r\n") == null); } for (options.privileged_headers) |header| { + assert(header.name.len != 0); assert(std.mem.indexOfPosLinear(u8, header.name, 0, "\r\n") == null); assert(std.mem.indexOfPosLinear(u8, header.value, 0, "\r\n") == null); } diff --git a/lib/std/http/HeaderIterator.zig b/lib/std/http/HeaderIterator.zig index 8d36374f8c..515058859d 100644 --- a/lib/std/http/HeaderIterator.zig +++ b/lib/std/http/HeaderIterator.zig @@ -15,7 +15,7 @@ pub fn next(it: *HeaderIterator) ?std.http.Header { var kv_it = std.mem.splitSequence(u8, it.bytes[it.index..end], ": "); const name = kv_it.next().?; const value = kv_it.rest(); - if (value.len == 0) { + if (name.len == 0 and value.len == 0) { if (it.is_trailer) return null; const next_end = std.mem.indexOfPosLinear(u8, it.bytes, end + 2, "\r\n") orelse return null; @@ -35,7 +35,7 @@ pub fn next(it: *HeaderIterator) ?std.http.Header { } test next { - var it = HeaderIterator.init("200 OK\r\na: b\r\nc: d\r\n\r\ne: f\r\n\r\n"); + var it = HeaderIterator.init("200 OK\r\na: b\r\nc: \r\nd: e\r\n\r\nf: g\r\n\r\n"); try std.testing.expect(!it.is_trailer); { const header = it.next().?; @@ -47,13 +47,19 @@ test next { const header = it.next().?; try std.testing.expect(!it.is_trailer); try std.testing.expectEqualStrings("c", header.name); - try std.testing.expectEqualStrings("d", header.value); + try std.testing.expectEqualStrings("", header.value); + } + { + const header = it.next().?; + try std.testing.expect(!it.is_trailer); + try std.testing.expectEqualStrings("d", header.name); + try std.testing.expectEqualStrings("e", header.value); } { const header = it.next().?; try std.testing.expect(it.is_trailer); - try std.testing.expectEqualStrings("e", header.name); - try std.testing.expectEqualStrings("f", header.value); + try std.testing.expectEqualStrings("f", header.name); + try std.testing.expectEqualStrings("g", header.value); } try std.testing.expectEqual(null, it.next()); } diff --git a/lib/std/http/Server.zig b/lib/std/http/Server.zig index 2d360d40a4..0454fa739e 100644 --- a/lib/std/http/Server.zig +++ b/lib/std/http/Server.zig @@ -211,7 +211,7 @@ pub const Request = struct { var line_it = mem.splitSequence(u8, line, ": "); const header_name = line_it.next().?; const header_value = line_it.rest(); - if (header_value.len == 0) return error.HttpHeadersInvalid; + if (header_name.len == 0) return error.HttpHeadersInvalid; if (std.ascii.eqlIgnoreCase(header_name, "connection")) { head.keep_alive = !std.ascii.eqlIgnoreCase(header_value, "close"); @@ -311,6 +311,7 @@ pub const Request = struct { assert(options.extra_headers.len <= max_extra_headers); if (std.debug.runtime_safety) { for (options.extra_headers) |header| { + assert(header.name.len != 0); assert(std.mem.indexOfScalar(u8, header.name, ':') == null); assert(std.mem.indexOfPosLinear(u8, header.name, 0, "\r\n") == null); assert(std.mem.indexOfPosLinear(u8, header.value, 0, "\r\n") == null); @@ -370,11 +371,13 @@ pub const Request = struct { }; iovecs_len += 1; - iovecs[iovecs_len] = .{ - .iov_base = header.value.ptr, - .iov_len = header.value.len, - }; - iovecs_len += 1; + if (header.value.len != 0) { + iovecs[iovecs_len] = .{ + .iov_base = header.value.ptr, + .iov_len = header.value.len, + }; + iovecs_len += 1; + } iovecs[iovecs_len] = .{ .iov_base = "\r\n", @@ -496,6 +499,7 @@ pub const Request = struct { } for (o.extra_headers) |header| { + assert(header.name.len != 0); h.appendSliceAssumeCapacity(header.name); h.appendSliceAssumeCapacity(": "); h.appendSliceAssumeCapacity(header.value); @@ -986,11 +990,13 @@ pub const Response = struct { }; iovecs_len += 1; - iovecs[iovecs_len] = .{ - .iov_base = trailer.value.ptr, - .iov_len = trailer.value.len, - }; - iovecs_len += 1; + if (trailer.value.len != 0) { + iovecs[iovecs_len] = .{ + .iov_base = trailer.value.ptr, + .iov_len = trailer.value.len, + }; + iovecs_len += 1; + } iovecs[iovecs_len] = .{ .iov_base = "\r\n", diff --git a/lib/std/http/test.zig b/lib/std/http/test.zig index e36b0cdf28..9343fd5a22 100644 --- a/lib/std/http/test.zig +++ b/lib/std/http/test.zig @@ -479,6 +479,12 @@ test "general client/server API coverage" { .{ .name = "location", .value = location }, }, }); + } else if (mem.eql(u8, request.head.target, "/empty")) { + try request.respond("", .{ + .extra_headers = &.{ + .{ .name = "empty", .value = "" }, + }, + }); } else { try request.respond("", .{ .status = .not_found }); } @@ -491,7 +497,10 @@ test "general client/server API coverage" { return s.listen_address.in.getPort(); } }); - defer test_server.destroy(); + defer { + global.handle_new_requests = false; + test_server.destroy(); + } const log = std.log.scoped(.client); @@ -654,6 +663,56 @@ test "general client/server API coverage" { // connection has been closed try expect(client.connection_pool.free_len == 0); + { // handle empty header field value + const location = try std.fmt.allocPrint(gpa, "http://127.0.0.1:{d}/empty", .{port}); + defer gpa.free(location); + const uri = try std.Uri.parse(location); + + log.info("{s}", .{location}); + var server_header_buffer: [1024]u8 = undefined; + var req = try client.open(.GET, uri, .{ + .server_header_buffer = &server_header_buffer, + .extra_headers = &.{ + .{ .name = "empty", .value = "" }, + }, + }); + defer req.deinit(); + + try req.send(.{}); + try req.wait(); + + try std.testing.expectEqual(.ok, req.response.status); + + const body = try req.reader().readAllAlloc(gpa, 8192); + defer gpa.free(body); + + try expectEqualStrings("", body); + + var it = req.response.iterateHeaders(); + { + const header = it.next().?; + try expect(!it.is_trailer); + try expectEqualStrings("connection", header.name); + try expectEqualStrings("keep-alive", header.value); + } + { + const header = it.next().?; + try expect(!it.is_trailer); + try expectEqualStrings("content-length", header.name); + try expectEqualStrings("0", header.value); + } + { + const header = it.next().?; + try expect(!it.is_trailer); + try expectEqualStrings("empty", header.name); + try expectEqualStrings("", header.value); + } + try expectEqual(null, it.next()); + } + + // connection has been kept alive + try expect(client.http_proxy != null or client.connection_pool.free_len == 1); + { // relative redirect const location = try std.fmt.allocPrint(gpa, "http://127.0.0.1:{d}/redirect/1", .{port}); defer gpa.free(location);