From d4c8c5f26e83d9518d2c30f7adfd3216b791a043 Mon Sep 17 00:00:00 2001 From: SatoshiQaziMuhammed Date: Tue, 20 Oct 2026 23:00:57 +0400 Subject: [PATCH] fix(ui): leave no angle bracket in a sanitized origin; judge markup on the raw one (#16) * fix(ui): leave no angle bracket in a sanitized origin; judge markup on the raw one sanitizeOrigin removed tags in a single pass, so `ipt>` came out as ``. validateOrigin looked for markup in the sanitized string, after the tags had been removed, so `x` passed as a valid origin. It now checks the raw origin first. pr-any.yml gets permissions: contents: read and lock.yml the issues/pull-requests/discussions write it needs (CodeQL actions/missing-workflow-permissions #3, #23). * test(ui): typed cases and the matchers the test shim implements The first version used it.each (untyped rows, @typescript-eslint/no-unsafe-argument) and expect().not.toMatch, which @pezkuwi/dev-test does not implement. Plain loops over string[] and a boolean assertion instead. With the repo runner: 5/5 pass on the fix, 3 of 5 fail on main's sanitizeOrigin.ts. --- .github/workflows/lock.yml | 6 +++ .github/workflows/pr-any.yml | 4 ++ .../src/util/sanitizeOrigin.spec.ts | 45 +++++++++++++++++++ .../extension-ui/src/util/sanitizeOrigin.ts | 11 ++++- 4 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 packages/extension-ui/src/util/sanitizeOrigin.spec.ts diff --git a/.github/workflows/lock.yml b/.github/workflows/lock.yml index 4901cb3..4a5b173 100644 --- a/.github/workflows/lock.yml +++ b/.github/workflows/lock.yml @@ -5,6 +5,12 @@ on: schedule: - cron: '20 2/3 * * *' + +permissions: + issues: write + pull-requests: write + discussions: write + jobs: lock: runs-on: ubuntu-latest diff --git a/.github/workflows/pr-any.yml b/.github/workflows/pr-any.yml index 0ed99b0..f4d9983 100644 --- a/.github/workflows/pr-any.yml +++ b/.github/workflows/pr-any.yml @@ -1,6 +1,10 @@ name: PR on: [pull_request] + +permissions: + contents: read + jobs: pr: strategy: diff --git a/packages/extension-ui/src/util/sanitizeOrigin.spec.ts b/packages/extension-ui/src/util/sanitizeOrigin.spec.ts new file mode 100644 index 0000000..3db1f8f --- /dev/null +++ b/packages/extension-ui/src/util/sanitizeOrigin.spec.ts @@ -0,0 +1,45 @@ +// Copyright 2019-2026 @pezkuwi/extension-ui authors & contributors +// SPDX-License-Identifier: Apache-2.0 + +import type * as _ from '@pezkuwi/dev-test/globals.d.ts'; + +import { sanitizeOrigin, validateOrigin } from './sanitizeOrigin.js'; + +// The origin a site claims for itself is attacker-chosen text. +const WITH_MARKUP: string[] = [ + 'ipt>alert(1)', + '<img src=x onerror=alert(1)>', + 'ay' +]; + +// validateOrigin used to look for markup in the sanitized string, after the +// tags it was looking for had been removed: `x` passed as valid. +const FLAGGED: string[] = ['ipt>evil', 'x']; + +const ACCEPTED: string[] = ['https://app.pezkuwichain.io', 'http://localhost:3000', 'Pezkuwi Apps']; + +describe('sanitizeOrigin', () => { + it('keeps a normal origin as it is', () => { + expect(sanitizeOrigin('Pezkuwi Apps')).toEqual('Pezkuwi Apps'); + }); + + // One pass of tag removal turns `ipt>` into ` { + for (const origin of WITH_MARKUP) { + expect(/[<>]/.test(sanitizeOrigin(origin))).toBe(false); + } + }); + + it('flags an origin that carried markup', () => { + for (const origin of FLAGGED) { + expect(validateOrigin(origin)).toBe(false); + } + }); + + it('accepts ordinary origins', () => { + for (const origin of ACCEPTED) { + expect(validateOrigin(origin)).toBe(true); + } + }); +}); diff --git a/packages/extension-ui/src/util/sanitizeOrigin.ts b/packages/extension-ui/src/util/sanitizeOrigin.ts index 2a5f7ff..c48e0c1 100644 --- a/packages/extension-ui/src/util/sanitizeOrigin.ts +++ b/packages/extension-ui/src/util/sanitizeOrigin.ts @@ -22,6 +22,11 @@ const sanitizeHtml = (input: string, options?: { .replace(/<[^>]*>/g, ''); // Remove other HTML tags } + // Tag removal is a single pass, so nested input such as `ipt>` + // leaves a new tag behind. Drop every remaining angle bracket; an origin never + // contains one, and validateOrigin already treats them as suspicious. + sanitized = sanitized.replace(/[<>]/g, ''); + // Normalize whitespace sanitized = sanitized .trim() @@ -44,6 +49,11 @@ export const sanitizeOrigin = (origin: string): string => { // Validate that the origin looks legitimate export const validateOrigin = (origin: string): boolean => { + // Markup is judged on what the site sent: the sanitized form no longer has any. + if (/[<>]/.test(origin)) { + return false; + } + const sanitized = sanitizeOrigin(origin); // Special case: always allow localhost URLs @@ -53,7 +63,6 @@ export const validateOrigin = (origin: string): boolean => { // Check for suspicious patterns const suspiciousPatterns = [ - /[<>]/, // HTML characters /^\s*$/, // Empty or whitespace only /.{50,}/, // Extremely long names /^(?!https?:\/\/).*:\/\//, // Any protocol other than http/https