Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 2 additions & 0 deletions docs/FileField.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import { FileField } from 'react-admin';

This field is also often used within a [`<FileInput />`](./FileInput.md) component to display preview.

**Tip**: To prevent XSS attacks, `<FileField>` doesn't render a clickable link when the file URL uses the `javascript:` protocol. It displays the title as plain text instead, and logs an error in the console.

## Properties

| Prop | Required | Type | Default | Description |
Expand Down
2 changes: 2 additions & 0 deletions docs/UrlField.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import { UrlField } from 'react-admin';
<UrlField source="site_url" />
```

**Tip**: To prevent XSS attacks, `<UrlField>` doesn't render a clickable link when the URL uses the `javascript:` protocol. It displays the URL as plain text instead, and logs an error in the console.

## `content`

You can customize the content of the link by passing a string as the `content` prop.
Expand Down
48 changes: 48 additions & 0 deletions packages/ra-core/src/util/getSafeUrl.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import expect from 'expect';
import { getSafeUrl, isSafeUrl } from './getSafeUrl';

describe('isSafeUrl', () => {
it('should accept safe URLs', () => {
expect(isSafeUrl('https://example.com')).toBe(true);
expect(isSafeUrl('http://example.com/path?query=1#hash')).toBe(true);
expect(isSafeUrl('mailto:john@example.com')).toBe(true);
expect(isSafeUrl('tel:+33123456789')).toBe(true);
expect(isSafeUrl('ftp://example.com/file.txt')).toBe(true);
expect(isSafeUrl('/posts/1')).toBe(true);
expect(isSafeUrl('posts/1')).toBe(true);
expect(isSafeUrl('#/posts/1')).toBe(true);
expect(isSafeUrl('//example.com')).toBe(true);
expect(isSafeUrl('data:application/pdf;base64,JVBERi0xLjQ=')).toBe(
true
);
});

it('should reject unsafe URLs', () => {
expect(isSafeUrl('javascript:alert(1)')).toBe(false);
expect(isSafeUrl('JavaScript:alert(1)')).toBe(false);
expect(isSafeUrl(' javascript:alert(1)')).toBe(false);
expect(isSafeUrl('java\tscript:alert(1)')).toBe(false);
expect(isSafeUrl('java\nscript:alert(1)')).toBe(false);
expect(isSafeUrl('\u0000javascript:alert(1)')).toBe(false);
});
});

describe('getSafeUrl', () => {
it('should return a safe URL without logging', () => {
const consoleError = jest.spyOn(console, 'error');
expect(getSafeUrl('https://example.com')).toBe('https://example.com');
expect(consoleError).not.toHaveBeenCalled();
consoleError.mockRestore();
});

it('should return undefined for an unsafe URL and log an error', () => {
const consoleError = jest
.spyOn(console, 'error')
.mockImplementation(() => {});
expect(getSafeUrl('javascript:alert(1)')).toBeUndefined();
expect(consoleError).toHaveBeenCalledWith(
'Blocked unsafe URL: "javascript:alert(1)"'
);
consoleError.mockRestore();
});
});
32 changes: 32 additions & 0 deletions packages/ra-core/src/util/getSafeUrl.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* Return the URL if it can be used as a link href without executing code,
* or undefined otherwise (and log an error)
*
* Rejects javascript: URLs, including obfuscated ones (mixed case,
* whitespace, control characters). Relative URLs are safe.
*
* @example
* getSafeUrl('https://example.com'); // 'https://example.com'
* getSafeUrl('/posts/1'); // '/posts/1'
* getSafeUrl('javascript:alert(1)'); // undefined
*/
export const getSafeUrl = (url: string): string | undefined => {
if (isSafeUrl(url)) {
return url;
}
console.error(`Blocked unsafe URL: ${JSON.stringify(url)}`);
return undefined;
};

/**
* Check that a URL can be used as a link href without executing code
*
* @see getSafeUrl
*/
export const isSafeUrl = (url: string): boolean => {
try {
return new URL(url, window.location.href).protocol !== 'javascript:';
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
} catch {
return false;
}
};
1 change: 1 addition & 0 deletions packages/ra-core/src/util/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,4 @@ export * from './hooks';
export * from './shallowEqual';
export * from './useCheckForApplicationUpdate';
export * from './genericMemo';
export * from './getSafeUrl';
47 changes: 47 additions & 0 deletions packages/ra-ui-materialui/src/field/FileField.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,53 @@ describe('<FileField />', () => {
expect(secondLink.title).toEqual('Bye world!');
});

it('should not render a clickable link for javascript: URLs', () => {
const consoleError = jest
.spyOn(console, 'error')
.mockImplementation(() => {});
const { getByTitle } = render(
<FileField
{...defaultProps}
record={{ id: 123, url: 'javascript:alert(1)' }}
title="Click me"
/>
);
expect(getByTitle('Click me').getAttribute('href')).toBeNull();
expect(consoleError).toHaveBeenCalledWith(
'Blocked unsafe URL: "javascript:alert(1)"'
);
consoleError.mockRestore();
});

it('should not render clickable links for javascript: URLs in a list', () => {
const consoleError = jest
.spyOn(console, 'error')
.mockImplementation(() => {});
const { getByTitle } = render(
<FileField
{...defaultProps}
record={{
id: 123,
files: [
{ url: 'http://foo.com/bar.jpg', title: 'Safe' },
{ url: 'javascript:alert(1)', title: 'Unsafe' },
],
}}
source="files"
src="url"
title="title"
/>
);
expect(getByTitle('Safe').getAttribute('href')).toEqual(
'http://foo.com/bar.jpg'
);
expect(getByTitle('Unsafe').getAttribute('href')).toBeNull();
expect(consoleError).toHaveBeenCalledWith(
'Blocked unsafe URL: "javascript:alert(1)"'
);
consoleError.mockRestore();
});

it('should use custom className', () => {
const { container } = render(
<FileField
Expand Down
5 changes: 3 additions & 2 deletions packages/ra-ui-materialui/src/field/FileField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import Typography from '@mui/material/Typography';
import {
type ExtractRecordPaths,
type HintedString,
getSafeUrl,
useFieldValue,
useTranslate,
} from 'ra-core';
Expand Down Expand Up @@ -88,7 +89,7 @@ export const FileField = <
return (
<li key={index}>
<Link
href={srcValue}
href={getSafeUrl(srcValue)}
title={fileTitleValue}
target={target}
download={download}
Expand All @@ -109,7 +110,7 @@ export const FileField = <
return (
<Root className={className} {...sanitizeFieldRestProps(rest)}>
<Link
href={sourceValue?.toString()}
href={getSafeUrl(sourceValue.toString())}
title={titleValue}
target={target}
download={download}
Expand Down
21 changes: 20 additions & 1 deletion packages/ra-ui-materialui/src/field/UrlField.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { I18nContextProvider } from 'ra-core';
import polyglotI18nProvider from 'ra-i18n-polyglot';

import { UrlField } from './UrlField';
import { Themed } from './UrlField.stories';
import { Themed, UnsafeUrl } from './UrlField.stories';

const url = 'https://en.wikipedia.org/wiki/HAL_9000';

Expand Down Expand Up @@ -81,6 +81,25 @@ describe('<UrlField />', () => {
expect(getByText('Not found')).not.toBeNull();
});

it('should not render a clickable link for javascript: URLs', () => {
const consoleError = jest
.spyOn(console, 'error')
.mockImplementation(() => {});
render(<UnsafeUrl />);
const link = screen.getByText('javascript:alert(document.cookie)');
expect(link.getAttribute('href')).toBeNull();
expect(consoleError).toHaveBeenCalledWith(
'Blocked unsafe URL: "javascript:alert(document.cookie)"'
);
consoleError.mockRestore();
});

it('should render relative URLs as links', () => {
render(<UrlField record={{ id: 123, url: '/posts/1' }} source="url" />);
const link = screen.getByText('/posts/1') as HTMLAnchorElement;
expect(link.getAttribute('href')).toEqual('/posts/1');
});

it('should be customized by a theme', async () => {
render(<Themed />);
expect(screen.getByTestId('themed')).toBeDefined();
Expand Down
13 changes: 13 additions & 0 deletions packages/ra-ui-materialui/src/field/UrlField.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,16 @@ export const Themed = () => {
</AdminContext>
);
};

export const UnsafeUrl = () => (
<AdminContext>
<Paper sx={{ p: 2 }}>
<Stack direction="row">
<UrlField
record={{ value: 'javascript:alert(document.cookie)' }}
source="value"
/>
</Stack>
</Paper>
</AdminContext>
);
4 changes: 2 additions & 2 deletions packages/ra-ui-materialui/src/field/UrlField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
styled,
useThemeProps,
} from '@mui/material/styles';
import { useFieldValue, useTranslate, genericMemo } from 'ra-core';
import { useFieldValue, useTranslate, genericMemo, getSafeUrl } from 'ra-core';

import { FieldProps } from './types';

Expand Down Expand Up @@ -41,7 +41,7 @@ const UrlFieldImpl = <
return (
<StyledLink
className={className}
href={value}
href={getSafeUrl(value)}
onClick={stopPropagation}
variant="body2"
{...sanitizeFieldRestProps(rest)}
Expand Down
Loading