mirror of
https://github.com/pezkuwichain/pezkuwi-extension.git
synced 2026-10-30 07:09:49 +00:00
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 `<scr<script>ipt>` came out as `<script` (CodeQL js/incomplete-multi-character-sanitization #6, #7). The page renders the result as React text, so this was not exploitable here, but the helper's name promised more than it did. It now also drops every remaining `<` and `>`. validateOrigin looked for markup in the sanitized string, after the tags had been removed, so `<b>x</b>` 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.
This commit is contained in:
1 parent
c635e89c86
commit
d4c8c5f26e
4 files changed
+65
-1
No files matched your search
@@ -5,6 +5,12 @@ on:
|
||||
schedule:
|
||||
- cron: '20 2/3 * * *'
|
||||
|
||||
|
||||
permissions:
|
||||
issues: write
|
||||
pull-requests: write
|
||||
discussions: write
|
||||
|
||||
jobs:
|
||||
lock:
|
||||
runs-on: ubuntu-latest
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
name: PR
|
||||
on: [pull_request]
|
||||
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
jobs:
|
||||
pr:
|
||||
strategy:
|
||||
|
||||
@@ -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[] = [
|
||||
'<scr<script>ipt>alert(1)</script>',
|
||||
'<<b>img src=x onerror=alert(1)>',
|
||||
'a<b',
|
||||
'x>y'
|
||||
];
|
||||
|
||||
// validateOrigin used to look for markup in the sanitized string, after the
|
||||
// tags it was looking for had been removed: `<b>x</b>` passed as valid.
|
||||
const FLAGGED: string[] = ['<scr<script>ipt>evil', '<b>x</b>'];
|
||||
|
||||
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 `<scr<script>ipt>` into `<script`.
|
||||
it('leaves no angle bracket behind', () => {
|
||||
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);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -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 `<scr<script>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
|
||||
|
||||
Reference in new issue
Block a user