[GH-ISSUE #2601] fix(browser): reject malformed characters in SAZ XML metadata #917

New issue

Closed

opened 2026-10-15 18:29:33 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-15 18:29:33 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 13, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/2601

Summary

SAZ inspection accepts metadata containing an unescaped ampersand, a NUL character, or a surrogate character reference and returns those values in the record's reported XML tree. The source archive-decoder boundary reproduces this behavior even though malformed metadata is supposed to fail validation before record selection. This allows invalid producer metadata to appear in a successful capture result.

Steps to reproduce

Use the source checkout and locked dependencies described under Runtime or environment. Add the following focused test as src/browser/history/metadataValidity.test.ts, then run npm run test:focused -- src/browser/history/metadataValidity.test.ts. Each input fits in a single small metadata member and requires no live browser or Fiddler installation.

import { expect, it } from "vitest";
import { mkdtemp, writeFile, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { Uint8ArrayReader, Uint8ArrayWriter, ZipWriter } from "@zip.js/zip.js";
import { decodeSazCapture } from "./SazCapture.js";

it.each([
  "<Session>a & b</Session>",
  "<Session>a\x00b</Session>",
  "<Session>&#xD800;</Session>",
])("rejects malformed metadata %j", async (xml) => {
  const root = await mkdtemp(join(tmpdir(), "metadata-validity-"));
  try {
    const path = join(root, "capture.saz");
    const writer = new ZipWriter(new Uint8ArrayWriter(), { useWebWorkers: false });
    await writer.add("raw/1_m.xml", new Uint8ArrayReader(Buffer.from(xml)));
    await writeFile(path, Buffer.from(await writer.close()));
    await expect(decodeSazCapture(path, [])).rejects.toMatchObject({ reason: "format" });
  } finally {
    await rm(root, { recursive: true, force: true });
  }
});

An equivalent verification probe executed all three inputs, checked the returned metadata trees, and observed successful captures. The rejection assertions above describe the required behavior and would fail on those observed results.

Expected behavior

The selected archive should fail as malformed input when a metadata member is not well-formed XML. All records must pass validation before any selected subset is returned. Valid escaped text and CDATA should retain their ordinary values.

Actual behavior

Each of the three malformed documents returned a successful one-record capture at the source archive-decoder boundary. Its reported metadata contained respectively the text "a & b", "a\x00b", or the lone surrogate "\ud800". The parser rejected a mismatched closing-element control, while valid empty Session, escaped ampersand, and CDATA controls succeeded. Public CLI and MCP execution were not tested.

Evidence

Expected source: src/browser/history/SazMetadata.ts:32-50 defines the not-well-formed format failure; docs/web-network-captures.md:24-26 promises complete record validation before selection; tests/boundary/browser/sazWebCapture.test.ts:441-443 provides a malformed-XML rejection control.
Failure source: src/browser/history/SazMetadata.ts:23-31 admits all three inputs without a parser error, and src/browser/history/SazCapture.ts:127-145 accepts and projects their metadata into successful records. A focused archive-decoder probe directly observed those three returned trees.
Evidence provenance: observed
Local verification: reproduced
Reproduction completeness: complete

The four focused verification tests passed in 387 ms. The three archive cases returned one record each and retained their malformed character values; the fourth test checked rejection and valid-input controls. Python's Expat XML parser independently rejected the same three byte sequences. No external entity or DTD behavior is claimed by this report.

Restoration check

Require all three archive fixtures to fail with the existing malformed-input classification and no partial capture. Keep ordinary Session metadata, escaped ampersands, comments, CDATA, XML declarations, and supported BOM-marked UTF-16 metadata accepted. Keep mismatched elements rejected and preserve the existing unsupported-DTD outcome.

Runtime or environment

Source baseline 8b181716ac. Verification used Node.js v26.10.0, npm 11.19.1, and lockfile-installed dependencies, including the pinned XML parser.

Triage assessment

Impact level: P2
Assessment status: supported
Impact basis: Malformed metadata is accepted into successful reported evidence, violating complete input validation. The observed failure is limited to the three character forms tested; valid archives and malformed-element rejection controls retain their expected behavior.
Workaround status: unverified
Workaround basis: Independently validating each XML member before inspection could detect the malformed inputs, but a practical prevalidation workflow for affected callers was not verified.

Originally created by @coygeek on GitHub (Oct 13, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/2601 ## Summary SAZ inspection accepts metadata containing an unescaped ampersand, a NUL character, or a surrogate character reference and returns those values in the record's reported XML tree. The source archive-decoder boundary reproduces this behavior even though malformed metadata is supposed to fail validation before record selection. This allows invalid producer metadata to appear in a successful capture result. ## Steps to reproduce Use the source checkout and locked dependencies described under Runtime or environment. Add the following focused test as src/browser/history/metadataValidity.test.ts, then run `npm run test:focused -- src/browser/history/metadataValidity.test.ts`. Each input fits in a single small metadata member and requires no live browser or Fiddler installation. ```ts import { expect, it } from "vitest"; import { mkdtemp, writeFile, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { Uint8ArrayReader, Uint8ArrayWriter, ZipWriter } from "@zip.js/zip.js"; import { decodeSazCapture } from "./SazCapture.js"; it.each([ "<Session>a & b</Session>", "<Session>a\x00b</Session>", "<Session>&#xD800;</Session>", ])("rejects malformed metadata %j", async (xml) => { const root = await mkdtemp(join(tmpdir(), "metadata-validity-")); try { const path = join(root, "capture.saz"); const writer = new ZipWriter(new Uint8ArrayWriter(), { useWebWorkers: false }); await writer.add("raw/1_m.xml", new Uint8ArrayReader(Buffer.from(xml))); await writeFile(path, Buffer.from(await writer.close())); await expect(decodeSazCapture(path, [])).rejects.toMatchObject({ reason: "format" }); } finally { await rm(root, { recursive: true, force: true }); } }); ``` An equivalent verification probe executed all three inputs, checked the returned metadata trees, and observed successful captures. The rejection assertions above describe the required behavior and would fail on those observed results. ## Expected behavior The selected archive should fail as malformed input when a metadata member is not well-formed XML. All records must pass validation before any selected subset is returned. Valid escaped text and CDATA should retain their ordinary values. ## Actual behavior Each of the three malformed documents returned a successful one-record capture at the source archive-decoder boundary. Its reported metadata contained respectively the text "a & b", "a\x00b", or the lone surrogate "\ud800". The parser rejected a mismatched closing-element control, while valid empty Session, escaped ampersand, and CDATA controls succeeded. Public CLI and MCP execution were not tested. ## Evidence - Expected source: src/browser/history/SazMetadata.ts:32-50 defines the not-well-formed format failure; docs/web-network-captures.md:24-26 promises complete record validation before selection; tests/boundary/browser/sazWebCapture.test.ts:441-443 provides a malformed-XML rejection control. - Failure source: src/browser/history/SazMetadata.ts:23-31 admits all three inputs without a parser error, and src/browser/history/SazCapture.ts:127-145 accepts and projects their metadata into successful records. A focused archive-decoder probe directly observed those three returned trees. - Evidence provenance: observed - Local verification: reproduced - Reproduction completeness: complete The four focused verification tests passed in 387 ms. The three archive cases returned one record each and retained their malformed character values; the fourth test checked rejection and valid-input controls. Python's Expat XML parser independently rejected the same three byte sequences. No external entity or DTD behavior is claimed by this report. ## Restoration check Require all three archive fixtures to fail with the existing malformed-input classification and no partial capture. Keep ordinary Session metadata, escaped ampersands, comments, CDATA, XML declarations, and supported BOM-marked UTF-16 metadata accepted. Keep mismatched elements rejected and preserve the existing unsupported-DTD outcome. ## Runtime or environment Source baseline 8b181716aca2cb467be499cba226abfb5ae19141. Verification used Node.js v26.10.0, npm 11.19.1, and lockfile-installed dependencies, including the pinned XML parser. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: Malformed metadata is accepted into successful reported evidence, violating complete input validation. The observed failure is limited to the three character forms tested; valid archives and malformed-element rejection controls retain their expected behavior. - Workaround status: unverified - Workaround basis: Independently validating each XML member before inspection could detect the malformed inputs, but a practical prevalidation workflow for affected callers was not verified.

bsakaguchi

2026-10-15 18:29:33 +00:00

closed this issue
added the
area: runtime

bug
labels

bsakaguchi commented

2026-10-15 18:29:34 +00:00

Author

Owner

@morluto commented on GitHub (Oct 14, 2026):

Addressed on main in 29a2d13994708789fa3104437f1f2f93c6a34c36.

SAZ metadata is checked for XML 1.0 well-formedness by a strict SAX parser before DOM evidence projection. Malformed characters and references are rejected, while literal comment/CDATA text and valid entities remain preserved; DTDs remain unsupported.

Validation: Affected suites passed in the merged 65-file run (1169 passed, 2 explicit skips), with full static/architecture checks and documentation checks passing. Final installed-package verification is recorded separately in the audit.

Verification scope: Controlled source and production-boundary verification on macOS with pinned Node 24.18.0; real external engines and Windows hosts were not independently rerun. Closure covers the described source defect, not broader provider conformance or performance.

@morluto commented on GitHub (Oct 14, 2026): Addressed on main in [29a2d13994708789fa3104437f1f2f93c6a34c36](https://github.com/morluto/rea/commit/29a2d13994708789fa3104437f1f2f93c6a34c36). SAZ metadata is checked for XML 1.0 well-formedness by a strict SAX parser before DOM evidence projection. Malformed characters and references are rejected, while literal comment/CDATA text and valid entities remain preserved; DTDs remain unsupported. Validation: Affected suites passed in the merged 65-file run (1169 passed, 2 explicit skips), with full static/architecture checks and documentation checks passing. Final installed-package verification is recorded separately in the audit. Verification scope: Controlled source and production-boundary verification on macOS with pinned Node 24.18.0; real external engines and Windows hosts were not independently rerun. Closure covers the described source defect, not broader provider conformance or performance.

bsakaguchi referenced this issue

2026-10-16 21:10:14 +00:00

[GH-ISSUE #4500] [Bug] Electron inspection retains a script from an excluded srcdoc frame under a synthetic local-file sourceURL #1783

bsakaguchi referenced this issue

2026-10-16 21:17:37 +00:00

[PR #917] [MERGED] feat(browser): attribute native execution and listener sources #2345

bsakaguchi referenced this issue

2026-10-31 00:41:12 +00:00

[GH-ISSUE #5756] [Bug] observe-web-execution loses the target-termination reason when cleanup fails #3500