From 87c6d52abcf12837fa90924dd52af79c4abf1017 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Fri, 29 May 2026 09:32:03 +0800 Subject: [PATCH 1/4] Cleaner cookie ownership Previously, Cookie.Jar.add would only conditionally take over the cookie. The caller had no way to know whether or not to deinit it. This could result in a double-free on certain error paths. Cookie.Jar.add now unconditionally takes ownership of the cookie. --- src/browser/webapi/HTMLDocument.zig | 1 - src/browser/webapi/storage/Cookie.zig | 12 ++--- src/browser/webapi/storage/CookieStore.zig | 52 ++++++++++++---------- src/cdp/domains/storage.zig | 46 ++++++++++--------- src/cookies.zig | 1 - 5 files changed, 60 insertions(+), 52 deletions(-) diff --git a/src/browser/webapi/HTMLDocument.zig b/src/browser/webapi/HTMLDocument.zig index e4b343fe..44e0ccfa 100644 --- a/src/browser/webapi/HTMLDocument.zig +++ b/src/browser/webapi/HTMLDocument.zig @@ -241,7 +241,6 @@ pub fn setCookie(_: *HTMLDocument, cookie_str: []const u8, frame: *Frame) ![]con // Invalid cookies should be silently ignored, not throw errors return ""; }; - errdefer c.deinit(); if (c.http_only) { c.deinit(); return ""; // HttpOnly cookies cannot be set from JS diff --git a/src/browser/webapi/storage/Cookie.zig b/src/browser/webapi/storage/Cookie.zig index 1bfa75b6..0e023705 100644 --- a/src/browser/webapi/storage/Cookie.zig +++ b/src/browser/webapi/storage/Cookie.zig @@ -484,10 +484,9 @@ pub const Jar = struct { /// Checks if addition comes from HTTP request or JS context. comptime is_http: bool, ) !void { - const is_expired = isCookieExpired(&cookie, request_time); - defer if (is_expired) { - cookie.deinit(); - }; + // `add` takes ownership of `cookie` unconditionally on entry. + var stored = false; + defer if (!stored) cookie.deinit(); if (self.cookies.items.len >= max_jar_size) { return error.CookieJarQuotaExceeded; @@ -496,6 +495,8 @@ pub const Jar = struct { return error.CookieSizeExceeded; } + const is_expired = isCookieExpired(&cookie, request_time); + for (self.cookies.items, 0..) |*c, i| { // We're only looking for the equal one. if (areCookiesEqual(&cookie, c) == false) { @@ -505,7 +506,6 @@ pub const Jar = struct { // RFC 6265bis 5.7.2: a non-HTTP API (e.g. document.cookie) must // not replace an HttpOnly cookie. if (c.http_only and is_http == false) { - if (is_expired == false) cookie.deinit(); return; } @@ -522,6 +522,7 @@ pub const Jar = struct { // after the assignment, c points at the new cookie. c.deinit(); self.cookies.items[i] = cookie; + stored = true; self.dispatchChange(.changed, &self.cookies.items[i]); } return; @@ -529,6 +530,7 @@ pub const Jar = struct { if (!is_expired) { try self.cookies.append(self.allocator, cookie); + stored = true; self.dispatchChange(.changed, &self.cookies.items[self.cookies.items.len - 1]); } } diff --git a/src/browser/webapi/storage/CookieStore.zig b/src/browser/webapi/storage/CookieStore.zig index 1e475286..4d679e54 100644 --- a/src/browser/webapi/storage/CookieStore.zig +++ b/src/browser/webapi/storage/CookieStore.zig @@ -427,34 +427,38 @@ fn storeCookie(exec: *const Execution, init: CookieInit) !void { if (!is_https) return error.InvalidPrefixedCookie; } - var arena = std.heap.ArenaAllocator.init(session.cookie_jar.allocator); - errdefer arena.deinit(); - const aa = arena.allocator(); + // The errdefer only protects construction failures. Once we `break :blk` + // with the Cookie value, `Jar.add` owns its lifetime. + const cookie: Cookie = blk: { + var arena = std.heap.ArenaAllocator.init(session.cookie_jar.allocator); + errdefer arena.deinit(); + const aa = arena.allocator(); - const owned_name = try aa.dupe(u8, init.name); - const owned_value = try aa.dupe(u8, init.value); - const owned_path = try Cookie.parsePath(aa, url, init.path); - const owned_domain = try Cookie.parseDomain(aa, url, init.domain); + const owned_name = try aa.dupe(u8, init.name); + const owned_value = try aa.dupe(u8, init.value); + const owned_path = try Cookie.parsePath(aa, url, init.path); + const owned_domain = try Cookie.parseDomain(aa, url, init.domain); - const cookie: Cookie = .{ - .arena = arena, - .name = owned_name, - .value = owned_value, - .path = owned_path, - .domain = owned_domain, + break :blk .{ + .arena = arena, + .name = owned_name, + .value = owned_value, + .path = owned_path, + .domain = owned_domain, - // CookieStore.expires is a unix timestamp in milliseconds; Cookie tracks - // expiry in seconds. A timestamp at or before "now" deletes the cookie via - // the Jar's expiry path. - .expires = if (init.expires) |ms| ms / 1000.0 else null, + // CookieStore.expires is a unix timestamp in milliseconds; Cookie tracks + // expiry in seconds. A timestamp at or before "now" deletes the cookie via + // the Jar's expiry path. + .expires = if (init.expires) |ms| ms / 1000.0 else null, - .secure = secure, - .http_only = false, - .same_site = switch (init.sameSite) { - .strict => .strict, - .lax => .lax, - .none => .none, - }, + .secure = secure, + .http_only = false, + .same_site = switch (init.sameSite) { + .strict => .strict, + .lax => .lax, + .none => .none, + }, + }; }; // CookieStore is a script API, so is_http = false. diff --git a/src/cdp/domains/storage.zig b/src/cdp/domains/storage.zig index 65a7509e..dac20e10 100644 --- a/src/cdp/domains/storage.zig +++ b/src/cdp/domains/storage.zig @@ -142,30 +142,34 @@ pub fn setCdpCookie(cookie_jar: *CookieJar, param: CdpCookie) !void { return error.NotImplemented; } - var arena = std.heap.ArenaAllocator.init(cookie_jar.allocator); - errdefer arena.deinit(); - const a = arena.allocator(); + // The errdefer only protects construction failures. Once we `break :blk` + // with the Cookie value, `Jar.add` owns its lifetime. + const cookie = blk: { + var arena = std.heap.ArenaAllocator.init(cookie_jar.allocator); + errdefer arena.deinit(); + const a = arena.allocator(); - // NOTE: The param.url can affect the default domain, (NOT path), secure, source port, and source scheme. - const domain = try Cookie.parseDomain(a, param.url, param.domain); - const path = if (param.path == null) "/" else try Cookie.parsePath(a, null, param.path); + // NOTE: The param.url can affect the default domain, (NOT path), secure, source port, and source scheme. + const domain = try Cookie.parseDomain(a, param.url, param.domain); + const path = if (param.path == null) "/" else try Cookie.parsePath(a, null, param.path); - const secure = if (param.secure) |s| s else if (param.url) |url| URL.isHTTPS(url) else false; + const secure = if (param.secure) |s| s else if (param.url) |url| URL.isHTTPS(url) else false; - const cookie = Cookie{ - .arena = arena, - .name = try a.dupe(u8, param.name), - .value = try a.dupe(u8, param.value), - .path = path, - .domain = domain, - .expires = param.expires, - .secure = secure, - .http_only = param.httpOnly, - .same_site = switch (param.sameSite) { - .Strict => .strict, - .Lax => .lax, - .None => .none, - }, + break :blk Cookie{ + .arena = arena, + .name = try a.dupe(u8, param.name), + .value = try a.dupe(u8, param.value), + .path = path, + .domain = domain, + .expires = param.expires, + .secure = secure, + .http_only = param.httpOnly, + .same_site = switch (param.sameSite) { + .Strict => .strict, + .Lax => .lax, + .None => .none, + }, + }; }; try cookie_jar.add(cookie, std.time.timestamp(), true); } diff --git a/src/cookies.zig b/src/cookies.zig index a654b279..3726f349 100644 --- a/src/cookies.zig +++ b/src/cookies.zig @@ -77,7 +77,6 @@ fn _loadFromFile(session: *Session, path: []const u8) !void { }; jar.add(cookie, now, true) catch |err| { - cookie.deinit(); log.warn(.app, "invalid cookie", .{ .name = jc.name, .err = err }); continue; }; From 23e58b005e00cabf0d226237a619440e495bbbbd Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Fri, 29 May 2026 11:30:14 +0800 Subject: [PATCH 2/4] Add Notification WebAPI Adds a pretty simplistic Notification WebAPI. Also adds a dummy drawImage to CanvasRenderingContext2D. Trying to improve how we're seen by https://bot.sannysoft.com/ --- src/browser/js/bridge.zig | 1 + .../canvas/canvas_rendering_context_2d.html | 13 ++ src/browser/tests/notification.html | 47 +++++ src/browser/webapi/EventTarget.zig | 3 + src/browser/webapi/Notification.zig | 175 ++++++++++++++++++ .../canvas/CanvasRenderingContext2D.zig | 5 + 6 files changed, 244 insertions(+) create mode 100644 src/browser/tests/notification.html create mode 100644 src/browser/webapi/Notification.zig diff --git a/src/browser/js/bridge.zig b/src/browser/js/bridge.zig index 2f9a073c..73c2a821 100644 --- a/src/browser/js/bridge.zig +++ b/src/browser/js/bridge.zig @@ -941,6 +941,7 @@ pub const PageJsApis = flattenTypes(&.{ @import("../webapi/ModelContext.zig"), @import("../webapi/Navigator.zig"), @import("../webapi/NavigatorUAData.zig"), + @import("../webapi/Notification.zig"), @import("../webapi/net/FormData.zig"), @import("../webapi/net/Headers.zig"), @import("../webapi/net/Request.zig"), diff --git a/src/browser/tests/canvas/canvas_rendering_context_2d.html b/src/browser/tests/canvas/canvas_rendering_context_2d.html index 66a673ad..060ca6c8 100644 --- a/src/browser/tests/canvas/canvas_rendering_context_2d.html +++ b/src/browser/tests/canvas/canvas_rendering_context_2d.html @@ -158,3 +158,16 @@ testing.expectEqual(null, element.getContext('webgl')); } + + diff --git a/src/browser/tests/notification.html b/src/browser/tests/notification.html new file mode 100644 index 00000000..667ef3d0 --- /dev/null +++ b/src/browser/tests/notification.html @@ -0,0 +1,47 @@ + + + + + + + + + + diff --git a/src/browser/webapi/EventTarget.zig b/src/browser/webapi/EventTarget.zig index 845537cd..d5d07310 100644 --- a/src/browser/webapi/EventTarget.zig +++ b/src/browser/webapi/EventTarget.zig @@ -50,6 +50,7 @@ pub const Type = union(enum) { font_face_set: *@import("css/FontFaceSet.zig"), websocket: *@import("net/WebSocket.zig"), cookie_store: *@import("storage/CookieStore.zig"), + notification: *@import("Notification.zig"), }; pub fn init(page: *Page) !*EventTarget { @@ -161,6 +162,7 @@ pub fn format(self: *EventTarget, writer: *std.Io.Writer) !void { .font_face_set => writer.writeAll(""), .websocket => writer.writeAll(""), .cookie_store => writer.writeAll(""), + .notification => writer.writeAll(""), }; } @@ -184,6 +186,7 @@ pub fn toString(self: *EventTarget) []const u8 { .font_face_set => return "[object FontFaceSet]", .websocket => return "[object WebSocket]", .cookie_store => return "[object CookieStore]", + .notification => return "[object Notification]", }; } diff --git a/src/browser/webapi/Notification.zig b/src/browser/webapi/Notification.zig new file mode 100644 index 00000000..c9ce3226 --- /dev/null +++ b/src/browser/webapi/Notification.zig @@ -0,0 +1,175 @@ +// Copyright (C) 2023-2026 Lightpanda (Selecy SAS) +// +// Francis Bouvier +// Pierre Tachoire +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as +// published by the Free Software Foundation, either version 3 of the +// License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +const std = @import("std"); +const lp = @import("lightpanda"); + +const js = @import("../js/js.zig"); +const Page = @import("../Page.zig"); + +const EventTarget = @import("EventTarget.zig"); + +const Execution = js.Execution; +const Allocator = std.mem.Allocator; + +const Notification = @This(); + +_rc: lp.RC(u8) = .{}, +_arena: Allocator, +_proto: *EventTarget, +_title: []const u8, +_body: []const u8 = "", +_icon: []const u8 = "", +_image: []const u8 = "", +_badge: []const u8 = "", +_tag: []const u8 = "", +_lang: []const u8 = "", +_dir: []const u8 = "auto", +_silent: bool = false, +_require_interaction: bool = false, +_renotify: bool = false, + +const Options = struct { + body: ?[]const u8 = null, + icon: ?[]const u8 = null, + image: ?[]const u8 = null, + badge: ?[]const u8 = null, + tag: ?[]const u8 = null, + lang: ?[]const u8 = null, + dir: ?[]const u8 = null, + silent: ?bool = null, + requireInteraction: ?bool = null, + renotify: ?bool = null, +}; + +pub fn init(title: []const u8, options_: ?Options, exec: *const Execution) !*Notification { + const arena = try exec.getArena(.small, "Notification"); + errdefer exec.releaseArena(arena); + + const options = options_ orelse Options{}; + return exec._factory.eventTargetWithAllocator(arena, Notification{ + ._arena = arena, + ._proto = undefined, + ._title = try arena.dupe(u8, title), + ._body = if (options.body) |v| try arena.dupe(u8, v) else "", + ._icon = if (options.icon) |v| try arena.dupe(u8, v) else "", + ._image = if (options.image) |v| try arena.dupe(u8, v) else "", + ._badge = if (options.badge) |v| try arena.dupe(u8, v) else "", + ._tag = if (options.tag) |v| try arena.dupe(u8, v) else "", + ._lang = if (options.lang) |v| try arena.dupe(u8, v) else "", + ._dir = if (options.dir) |d| try arena.dupe(u8, d) else "auto", + ._silent = options.silent orelse false, + ._require_interaction = options.requireInteraction orelse false, + ._renotify = options.renotify orelse false, + }); +} + +pub fn deinit(self: *Notification, page: *Page) void { + page.releaseArena(self._arena); +} + +pub fn releaseRef(self: *Notification, page: *Page) void { + self._rc.release(self, page); +} + +pub fn acquireRef(self: *Notification) void { + self._rc.acquire(); +} + +pub fn close(_: *Notification) void {} + +fn getPermission() []const u8 { + return "default"; +} + +fn getMaxActions() u32 { + return 2; +} + +fn requestPermission(_: ?js.Function, exec: *const Execution) !js.Promise { + return exec.js.local.?.resolvePromise("default"); +} + +fn getTitle(self: *const Notification) []const u8 { + return self._title; +} +fn getBody(self: *const Notification) []const u8 { + return self._body; +} +fn getIcon(self: *const Notification) []const u8 { + return self._icon; +} +fn getImage(self: *const Notification) []const u8 { + return self._image; +} +fn getBadge(self: *const Notification) []const u8 { + return self._badge; +} +fn getTag(self: *const Notification) []const u8 { + return self._tag; +} +fn getLang(self: *const Notification) []const u8 { + return self._lang; +} +fn getDir(self: *const Notification) []const u8 { + return self._dir; +} +fn getSilent(self: *const Notification) bool { + return self._silent; +} +fn getRequireInteraction(self: *const Notification) bool { + return self._require_interaction; +} +fn getRenotify(self: *const Notification) bool { + return self._renotify; +} + +pub const JsApi = struct { + pub const bridge = js.Bridge(Notification); + + pub const Meta = struct { + pub const name = "Notification"; + pub const prototype_chain = bridge.prototypeChain(); + pub var class_id: bridge.ClassId = undefined; + }; + + pub const constructor = bridge.constructor(Notification.init, .{}); + + pub const permission = bridge.accessor(getPermission, null, .{ .static = true }); + pub const maxActions = bridge.accessor(getMaxActions, null, .{ .static = true }); + pub const requestPermission = bridge.function(Notification.requestPermission, .{ .static = true }); + + pub const close = bridge.function(Notification.close, .{ .noop = true }); + + pub const title = bridge.accessor(getTitle, null, .{}); + pub const body = bridge.accessor(getBody, null, .{}); + pub const icon = bridge.accessor(getIcon, null, .{}); + pub const image = bridge.accessor(getImage, null, .{}); + pub const badge = bridge.accessor(getBadge, null, .{}); + pub const tag = bridge.accessor(getTag, null, .{}); + pub const lang = bridge.accessor(getLang, null, .{}); + pub const dir = bridge.accessor(getDir, null, .{}); + pub const silent = bridge.accessor(getSilent, null, .{}); + pub const requireInteraction = bridge.accessor(getRequireInteraction, null, .{}); + pub const renotify = bridge.accessor(getRenotify, null, .{}); +}; + +const testing = @import("../../testing.zig"); +test "WebApi: Notification" { + try testing.htmlRunner("notification.html", .{}); +} diff --git a/src/browser/webapi/canvas/CanvasRenderingContext2D.zig b/src/browser/webapi/canvas/CanvasRenderingContext2D.zig index e1018c19..68d5ec6c 100644 --- a/src/browser/webapi/canvas/CanvasRenderingContext2D.zig +++ b/src/browser/webapi/canvas/CanvasRenderingContext2D.zig @@ -83,6 +83,10 @@ pub fn createImageData( pub fn putImageData(_: *const CanvasRenderingContext2D, _: *ImageData, _: f64, _: f64, _: ?f64, _: ?f64, _: ?f64, _: ?f64) void {} +// CanvasImageSource (HTMLImageElement, HTMLCanvasElement, ImageBitmap, ...) is +// just taken as a js.Value for now since we don't use it, and that's much easier. +pub fn drawImage(_: *const CanvasRenderingContext2D, _: js.Value, _: f64, _: f64, _: ?f64, _: ?f64, _: ?f64, _: ?f64, _: ?f64, _: ?f64) void {} + pub fn getImageData( _: *const CanvasRenderingContext2D, _: i32, // sx @@ -150,6 +154,7 @@ pub const JsApi = struct { pub const createImageData = bridge.function(CanvasRenderingContext2D.createImageData, .{ .dom_exception = true }); pub const putImageData = bridge.function(CanvasRenderingContext2D.putImageData, .{ .noop = true }); + pub const drawImage = bridge.function(CanvasRenderingContext2D.drawImage, .{ .noop = true }); pub const getImageData = bridge.function(CanvasRenderingContext2D.getImageData, .{ .dom_exception = true }); pub const save = bridge.function(CanvasRenderingContext2D.save, .{ .noop = true }); pub const restore = bridge.function(CanvasRenderingContext2D.restore, .{ .noop = true }); From a0c86df76748935f6d936a0136193aa1ea0e0aa4 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Fri, 29 May 2026 12:21:52 +0800 Subject: [PATCH 3/4] Prevent double-free on Synthetic URL If a synthetic url (blob URL) causes a navigation event, the frame abort will deinit the transfer, causing the `defer transfer.deinit()` atop Synthetic.run from firing. Flag the transfer as .completing to prevent this from happening. This mimics what non-synthetic urls do. --- src/browser/HttpClient.zig | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/browser/HttpClient.zig b/src/browser/HttpClient.zig index a45b6b6c..81c4dc14 100644 --- a/src/browser/HttpClient.zig +++ b/src/browser/HttpClient.zig @@ -602,6 +602,9 @@ const Synthetic = struct { } fn run(transfer: *Transfer, _: *anyopaque) void { + // prevents a callback that triggers a navigation queue from killing + // this transfer from under us. + transfer.state = .completing; defer transfer.deinit(); const fulfilled = build(transfer) catch |err| { From e6332ac121b35a1f26c0f2e49f7a5cdee4d8fb83 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Sat, 30 May 2026 08:44:35 +0800 Subject: [PATCH 4/4] Close session before freeing notification With the new CookieStore, the session must be freed before the notification is. This is how it works in CDP, but in fetch, we were pretty lazy about it. This caused the notification to be freed first, and then the cookiestore to try to unregister: UAF. --- src/lightpanda.zig | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lightpanda.zig b/src/lightpanda.zig index 155ccd4b..83214b4c 100644 --- a/src/lightpanda.zig +++ b/src/lightpanda.zig @@ -67,6 +67,7 @@ pub fn fetch(app: *App, browser: *Browser, url: [:0]const u8, opts: FetchOpts) ! defer notification.deinit(); var session = try browser.newSession(notification); + defer browser.closeSession(); if (app.config.cookieFile()) |cookie_path| { cookies.loadFromFile(session, cookie_path);