Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/reference/router.md
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,10 @@ import { Link } from '@askrjs/askr/router';

`Link` accepts normal renderable child content. Imperative DOM `Node` children are not a supported public contract.

Raw `href` values may be relative URLs or use `http`, `https`, `mailto`,
`sms`, or `tel`. `Link` rejects other explicit schemes, including executable
and local-file URLs.

## Types

| Type | Description |
Expand Down
8 changes: 8 additions & 0 deletions src/common/url.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
const SAFE_URL_SCHEMES = new Set(['http', 'https', 'mailto', 'sms', 'tel']);
const URL_SCHEME_RE = /^([a-z][a-z0-9+.-]*):/i;

export function isSafeHref(value: string): boolean {
const compact = value.trim().replace(/[\u0000-\u0020\u007f-\u009f]/g, '');

Check warning on line 5 in src/common/url.ts

View workflow job for this annotation

GitHub Actions / CI (ubuntu-latest)

eslint(no-control-regex)

Unexpected control character
const scheme = URL_SCHEME_RE.exec(compact)?.[1]?.toLowerCase();
return scheme === undefined || SAFE_URL_SCHEMES.has(scheme);
}
4 changes: 4 additions & 0 deletions src/components/link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { navigate } from '../router/navigate';
import type { RouteDestination } from '../common/router';
import { applyInteractionPolicy } from '../foundations/interactions';
import { mergeProps } from '../foundations/utilities';
import { isSafeHref } from '../common/url';

type LinkBaseProps = Omit<
Props,
Expand Down Expand Up @@ -114,6 +115,9 @@ export function Link({
}: LinkProps): JSXElement {
const href = to?.href ?? suppliedHref;
if (!href) throw new Error('Link requires href or to.');
if (!isSafeHref(href)) {
throw new TypeError('Link href uses an unsafe URL scheme.');
}
Comment thread
smiggleworth marked this conversation as resolved.
const handleNavigation = (e: Event) => {
if (e.defaultPrevented) {
return;
Expand Down
5 changes: 5 additions & 0 deletions src/renderer/attributes.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { Props } from '../common/props';
import { sanitizeCssValue } from '../common/css';
import { isSafeHref } from '../common/url';
import { incrementPerfMetric } from '../runtime';
import {
extractKey,
Expand Down Expand Up @@ -217,6 +218,8 @@ export function applyStaticScalarPropsToElement(
applyStylePropValue(el, value);
} else if (key === 'value' || key === 'checked') {
applyFormControlProp(el, key, value, tagName);
} else if (key.toLowerCase() === 'href' && !isSafeHref(String(value))) {
removeRenderedAttribute(el, key);
} else {
setRenderedAttribute(el, key, String(value));
}
Expand Down Expand Up @@ -357,6 +360,8 @@ export function applyScalarPropValue(
applyStylePropValue(el, value);
} else if (key === 'value' || key === 'checked') {
applyFormControlProp(el, key, value, tagName);
} else if (key.toLowerCase() === 'href' && !isSafeHref(String(value))) {
removeRenderedAttribute(el, key);
} else {
setRenderedAttribute(el, key, String(value));
}
Expand Down
3 changes: 3 additions & 0 deletions src/ssr/attrs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import type { Props } from '../common/props';
import { getPublicAttributeName } from '../common/attr-names';
import { isSafeHref } from '../common/url';
import type { RenderSink } from './sink';
import { escapeAttr, needsEscapeAttr, styleObjToCss } from './escape';

Expand Down Expand Up @@ -107,6 +108,7 @@ export function renderAttrsDirect(

// Regular attributes
const strValue = String(value);
if (attrName.toLowerCase() === 'href' && !isSafeHref(strValue)) continue;
sink.write(' ');
sink.write(attrName);
sink.write('="');
Expand Down Expand Up @@ -182,6 +184,7 @@ export function renderAttrs(
continue;
} else {
const strValue = String(value);
if (attrName.toLowerCase() === 'href' && !isSafeHref(strValue)) continue;
attrParts.push(` ${attrName}="${getEscapedAttrValue(strValue)}"`);
}
}
Expand Down
72 changes: 72 additions & 0 deletions tests/jsdom/renderer/unsafe-href.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import { describe, expect, it } from 'vite-plus/test';
import { Link } from '../../../src/components/link';
import { renderToStringSync } from '../../../src/ssr';
import { createIsland } from '../../../test-utils/render/create-island';
import {
createTestContainer,
flushScheduler,
} from '../../../test-utils/render/test-renderer';

describe('unsafe href schemes', () => {
it.each([
'javascript:alert(1)',
'java\nscript:alert(1)',
'data:text/html,phish',
'vbscript:msgbox(1)',
'file:///etc/passwd',
])('should reject unsafe Link href %s', (href) => {
expect(() => Link({ href, children: 'unsafe' })).toThrow(
'unsafe URL scheme'
);
});

it('should omit unsafe intrinsic anchor hrefs in client and SSR output', () => {
const { container, cleanup } = createTestContainer();
try {
createIsland({
root: container,
component: () => <a href="javascript:alert(1)">unsafe</a>,
});
flushScheduler();
expect(container.querySelector('a')?.hasAttribute('href')).toBe(false);

const html = renderToStringSync(
() => <a href="data:text/html,phish">unsafe</a>,
{}
);
expect(html).toBe('<a>unsafe</a>');
Comment thread
smiggleworth marked this conversation as resolved.
} finally {
cleanup();
}
});

it('should reject case-insensitive intrinsic href attribute names', () => {
const unsafeProps = { HREF: 'javascript:alert(1)' };
const { container, cleanup } = createTestContainer();
try {
createIsland({
root: container,
component: () => <a {...unsafeProps}>unsafe</a>,
});
flushScheduler();
expect(container.querySelector('a')?.hasAttribute('href')).toBe(false);

const html = renderToStringSync(() => <a {...unsafeProps}>unsafe</a>, {});
expect(html).toBe('<a>unsafe</a>');
} finally {
cleanup();
}
});

it('should preserve relative, web, mail, and telephone hrefs', () => {
for (const href of [
'/docs',
'#section',
'https://example.test/docs',
'mailto:[email protected]',
'tel:+15551234567',
]) {
expect(() => Link({ href, children: 'safe' })).not.toThrow();
}
});
});
Loading