From a3dd3f254dd5e8ccbfae52d030068a01714ba3c6 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 13 Jul 2026 07:36:16 +0800 Subject: [PATCH] mem: Make Request, URLSearchParams finalized Also, fix potential UAF in FormData iterator --- src/browser/webapi/URL.zig | 7 ++- src/browser/webapi/net/Fetch.zig | 4 ++ src/browser/webapi/net/FormData.zig | 17 ++++-- src/browser/webapi/net/Request.zig | 44 +++++++++++--- src/browser/webapi/net/URLSearchParams.zig | 70 +++++++++++++++++----- 5 files changed, 111 insertions(+), 31 deletions(-) diff --git a/src/browser/webapi/URL.zig b/src/browser/webapi/URL.zig index ace7d6796..b130275e5 100644 --- a/src/browser/webapi/URL.zig +++ b/src/browser/webapi/URL.zig @@ -58,7 +58,10 @@ pub fn parse(url: []const u8, maybe_base: ?[]const u8, exec: *const Execution) ? return URL.init(url, maybe_base, exec) catch null; } -pub fn deinit(self: *URL, _: *Page) void { +pub fn deinit(self: *URL, page: *Page) void { + if (self._search_params) |search_params| { + search_params.releaseRef(page); + } // Not tracked by arena. U.url_free(self._url); } @@ -254,6 +257,8 @@ pub fn getSearchParams(self: *URL, exec: *const Execution) !*URLSearchParams { const search_value = if (U.url_get_query(self._url, &out, &len) == 0) (out - 1)[0 .. len + 1] else ""; const params = try URLSearchParams.init(.{ .query_string = search_value }, exec); + // Released in deinit; the cached params must outlive their JS wrapper. + params.acquireRef(); self._search_params = params; return params; } diff --git a/src/browser/webapi/net/Fetch.zig b/src/browser/webapi/net/Fetch.zig index 86e53ca4b..4ba7edcac 100644 --- a/src/browser/webapi/net/Fetch.zig +++ b/src/browser/webapi/net/Fetch.zig @@ -55,6 +55,10 @@ pub fn init(input: Input, options: ?InitOpts, exec: *const Execution) !js.Promis resolver.rejectError("fetch init error", .{ .type_error = "Failed to construct Request" }); return resolver.promise(); }; + // This Request is never exposed to JS. makeRequest dupes the url/body + // into the transfer, so nothing references it once we return. + request.acquireRef(); + defer request.releaseRef(exec.page); if (request._signal) |signal| { if (signal._aborted) { diff --git a/src/browser/webapi/net/FormData.zig b/src/browser/webapi/net/FormData.zig index f70a6545c..3cf0174c9 100644 --- a/src/browser/webapi/net/FormData.zig +++ b/src/browser/webapi/net/FormData.zig @@ -199,15 +199,15 @@ fn deleteByName(self: *FormData, name: String, exec: *Execution) void { } pub fn keys(self: *FormData, exec: *const js.Execution) !*KeyIterator { - return KeyIterator.init(.{ .fd = self, .list = self }, exec); + return KeyIterator.init(.{ .fd = self }, exec); } pub fn values(self: *FormData, exec: *const js.Execution) !*ValueIterator { - return ValueIterator.init(.{ .fd = self, .list = self }, exec); + return ValueIterator.init(.{ .fd = self }, exec); } pub fn entries(self: *FormData, exec: *const js.Execution) !*EntryIterator { - return EntryIterator.init(.{ .fd = self, .list = self }, exec); + return EntryIterator.init(.{ .fd = self }, exec); } pub fn forEach(self: *FormData, cb_: js.Function, js_this_: ?js.Object) !void { @@ -350,11 +350,16 @@ pub const Iterator = struct { index: u32 = 0, fd: *FormData, - // See KeyValueList.Iterator.list — required by the GenericIterator wrapper. - list: *anyopaque, - pub const Entry = struct { []const u8, []const u8 }; + pub fn acquireRef(self: *Iterator) void { + self.fd.acquireRef(); + } + + pub fn releaseRef(self: *Iterator, page: *Page) void { + self.fd.releaseRef(page); + } + pub fn next(self: *Iterator, _: *const Execution) ?Iterator.Entry { const index = self.index; const items = self.fd._entries.items; diff --git a/src/browser/webapi/net/Request.zig b/src/browser/webapi/net/Request.zig index 7faca42cc..f8c5ac6f9 100644 --- a/src/browser/webapi/net/Request.zig +++ b/src/browser/webapi/net/Request.zig @@ -17,11 +17,13 @@ // along with this program. If not, see . const std = @import("std"); +const lp = @import("lightpanda"); const js = @import("../../js/js.zig"); const http = @import("../../../network/http.zig"); const URL = @import("../URL.zig"); +const Page = @import("../../Page.zig"); const Blob = @import("../Blob.zig"); const AbortSignal = @import("../AbortSignal.zig"); @@ -34,6 +36,7 @@ const Allocator = std.mem.Allocator; const Request = @This(); +_rc: lp.RC(u8) = .{}, _url: [:0]const u8, _method: http.Method, _headers: ?*Headers, @@ -88,7 +91,9 @@ const Cache = enum { }; pub fn init(input: Input, opts_: ?InitOpts, exec: *const Execution) !*Request { - const arena = exec.arena; + const arena = try exec.getArena(.medium, "Request"); + errdefer exec.releaseArena(arena); + const url = switch (input) { .url => |u| try URL.resolve(arena, exec.base(), u, .{ .encoding = exec.charset.* }), .request => |r| try arena.dupeZ(u8, r._url), @@ -130,7 +135,9 @@ pub fn init(input: Input, opts_: ?InitOpts, exec: *const Execution) !*Request { break :blk extracted.bytes; } else switch (input) { .url => null, - .request => |r| r._body, + // Dupe: the source Request owns its body bytes and may be finalized + // before this one. + .request => |r| if (r._body) |b| try arena.dupe(u8, b) else null, }; const signal = if (opts.signal) |s| @@ -140,7 +147,8 @@ pub fn init(input: Input, opts_: ?InitOpts, exec: *const Execution) !*Request { .request => |r| r._signal, }; - return exec._factory.create(Request{ + const self = try arena.create(Request); + self.* = .{ ._url = url, ._arena = arena, ._method = method, @@ -150,7 +158,20 @@ pub fn init(input: Input, opts_: ?InitOpts, exec: *const Execution) !*Request { ._redirect = opts.redirect, ._body = body, ._signal = signal, - }); + }; + return self; +} + +pub fn deinit(self: *Request, page: *Page) void { + page.releaseArena(self._arena); +} + +pub fn releaseRef(self: *Request, page: *Page) void { + self._rc.release(self, page); +} + +pub fn acquireRef(self: *Request) void { + self._rc.acquire(); } fn parseMethod(method: []const u8, exec: *const Execution) !http.Method { @@ -278,17 +299,22 @@ pub fn bytes(self: *Request, exec: *const Execution) !js.Promise { } pub fn clone(self: *const Request, exec: *const Execution) !*Request { - return exec._factory.create(Request{ - ._url = self._url, - ._arena = self._arena, + const arena = try exec.getArena(if (self._body) |b| b.len else 512, "Request.clone"); + errdefer exec.releaseArena(arena); + + const request = try arena.create(Request); + request.* = .{ + ._url = try arena.dupeZ(u8, self._url), + ._arena = arena, ._method = self._method, ._headers = self._headers, ._cache = self._cache, ._credentials = self._credentials, ._redirect = self._redirect, - ._body = self._body, + ._body = if (self._body) |b| try arena.dupe(u8, b) else null, ._signal = self._signal, - }); + }; + return request; } pub const JsApi = struct { diff --git a/src/browser/webapi/net/URLSearchParams.zig b/src/browser/webapi/net/URLSearchParams.zig index f0dc14eca..dbb1e09ee 100644 --- a/src/browser/webapi/net/URLSearchParams.zig +++ b/src/browser/webapi/net/URLSearchParams.zig @@ -20,6 +20,7 @@ const std = @import("std"); const lp = @import("lightpanda"); const js = @import("../../js/js.zig"); +const Page = @import("../../Page.zig"); const FormData = @import("FormData.zig"); const KeyValueList = @import("../KeyValueList.zig"); @@ -29,8 +30,18 @@ const String = lp.String; const Execution = js.Execution; const Allocator = std.mem.Allocator; +pub fn registerTypes() []const type { + return &.{ + URLSearchParams, + KeyIterator, + ValueIterator, + EntryIterator, + }; +} + const URLSearchParams = @This(); +_rc: lp.RC(u8) = .{}, _arena: Allocator, _params: KeyValueList, @@ -41,7 +52,9 @@ const InitOpts = union(enum) { }; pub fn init(opts_: ?InitOpts, exec: *const Execution) !*URLSearchParams { - const arena = exec.arena; + const arena = try exec.getArena(.small, "URLSearchParams"); + errdefer exec.releaseArena(arena); + const params: KeyValueList = blk: { const opts = opts_ orelse break :blk .empty; switch (opts) { @@ -73,10 +86,24 @@ pub fn init(opts_: ?InitOpts, exec: *const Execution) !*URLSearchParams { } }; - return exec._factory.create(URLSearchParams{ + const self = try arena.create(URLSearchParams); + self.* = .{ ._arena = arena, ._params = params, - }); + }; + return self; +} + +pub fn deinit(self: *URLSearchParams, page: *Page) void { + page.releaseArena(self._arena); +} + +pub fn releaseRef(self: *URLSearchParams, page: *Page) void { + self._rc.release(self, page); +} + +pub fn acquireRef(self: *URLSearchParams) void { + self._rc.acquire(); } pub fn updateFromString(self: *URLSearchParams, query_string: []const u8, exec: *const Execution) !void { @@ -111,16 +138,16 @@ pub fn delete(self: *URLSearchParams, name: []const u8, value: ?[]const u8) void self._params.delete(name, value); } -pub fn keys(self: *URLSearchParams, exec: *const Execution) !*KeyValueList.KeyIterator { - return KeyValueList.KeyIterator.init(.{ .list = self, .kv = &self._params }, exec); +pub fn keys(self: *URLSearchParams, exec: *const Execution) !*KeyIterator { + return KeyIterator.init(.{ .list = self }, exec); } -pub fn values(self: *URLSearchParams, exec: *const Execution) !*KeyValueList.ValueIterator { - return KeyValueList.ValueIterator.init(.{ .list = self, .kv = &self._params }, exec); +pub fn values(self: *URLSearchParams, exec: *const Execution) !*ValueIterator { + return ValueIterator.init(.{ .list = self }, exec); } -pub fn entries(self: *URLSearchParams, exec: *const Execution) !*KeyValueList.EntryIterator { - return KeyValueList.EntryIterator.init(.{ .list = self, .kv = &self._params }, exec); +pub fn entries(self: *URLSearchParams, exec: *const Execution) !*EntryIterator { + return EntryIterator.init(.{ .list = self }, exec); } pub fn toString(self: *const URLSearchParams, writer: *std.Io.Writer) !void { @@ -295,23 +322,36 @@ inline fn decodeHex(char: u8) u8 { pub const Iterator = struct { index: u32 = 0, - list: *const URLSearchParams, + list: *URLSearchParams, - const Entry = struct { []const u8, []const u8 }; + pub const Entry = struct { []const u8, []const u8 }; - pub fn next(self: *Iterator, _: *const Execution) !?Iterator.Entry { + pub fn acquireRef(self: *Iterator) void { + self.list.acquireRef(); + } + + pub fn releaseRef(self: *Iterator, page: *Page) void { + self.list.releaseRef(page); + } + + pub fn next(self: *Iterator, _: *const Execution) ?Iterator.Entry { const index = self.index; - const items = self.list._params.items; - if (index >= items.len) { + const entries_ = self.list._params._entries.items; + if (index >= entries_.len) { return null; } self.index = index + 1; - const e = &items[index]; + const e = &entries_[index]; return .{ e.name.str(), e.value.str() }; } }; +const GenericIterator = @import("../collections/iterator.zig").Entry; +pub const KeyIterator = GenericIterator(Iterator, "0"); +pub const ValueIterator = GenericIterator(Iterator, "1"); +pub const EntryIterator = GenericIterator(Iterator, null); + pub const JsApi = struct { pub const bridge = js.Bridge(URLSearchParams);