diff --git a/docs/FileField.md b/docs/FileField.md index 9c767928461..af2f9d54b31 100644 --- a/docs/FileField.md +++ b/docs/FileField.md @@ -20,6 +20,8 @@ import { FileField } from 'react-admin'; This field is also often used within a [``](./FileInput.md) component to display preview. +**Tip**: To prevent XSS attacks, `` 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 | diff --git a/docs/UrlField.md b/docs/UrlField.md index bdc633a0284..3485cea3374 100644 --- a/docs/UrlField.md +++ b/docs/UrlField.md @@ -14,6 +14,8 @@ import { UrlField } from 'react-admin'; ``` +**Tip**: To prevent XSS attacks, `` 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. diff --git a/packages/ra-core/src/util/getSafeUrl.spec.ts b/packages/ra-core/src/util/getSafeUrl.spec.ts new file mode 100644 index 00000000000..7b19a7024fb --- /dev/null +++ b/packages/ra-core/src/util/getSafeUrl.spec.ts @@ -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(); + }); +}); diff --git a/packages/ra-core/src/util/getSafeUrl.ts b/packages/ra-core/src/util/getSafeUrl.ts new file mode 100644 index 00000000000..6e4e18ba601 --- /dev/null +++ b/packages/ra-core/src/util/getSafeUrl.ts @@ -0,0 +1,33 @@ +/** + * 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 => + !isJavaScriptProtocol.test(url); + +// A javascript: URL can contain leading C0 control characters or spaces, +// and tabs or newlines anywhere (https://url.spec.whatwg.org/#url-parsing) +const isJavaScriptProtocol = + // eslint-disable-next-line no-control-regex + /^[\u0000-\u001F ]*j[\r\n\t]*a[\r\n\t]*v[\r\n\t]*a[\r\n\t]*s[\r\n\t]*c[\r\n\t]*r[\r\n\t]*i[\r\n\t]*p[\r\n\t]*t[\r\n\t]*:/i; diff --git a/packages/ra-core/src/util/index.ts b/packages/ra-core/src/util/index.ts index 0ea9d5fc836..170edf46cf0 100644 --- a/packages/ra-core/src/util/index.ts +++ b/packages/ra-core/src/util/index.ts @@ -29,3 +29,4 @@ export * from './hooks'; export * from './shallowEqual'; export * from './useCheckForApplicationUpdate'; export * from './genericMemo'; +export * from './getSafeUrl'; diff --git a/packages/ra-ui-materialui/src/field/FileField.spec.tsx b/packages/ra-ui-materialui/src/field/FileField.spec.tsx index 1d40d859be5..2644773be64 100644 --- a/packages/ra-ui-materialui/src/field/FileField.spec.tsx +++ b/packages/ra-ui-materialui/src/field/FileField.spec.tsx @@ -173,6 +173,53 @@ describe('', () => { 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( + + ); + 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( + + ); + 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( ', () => { expect(getByText('Not found')).not.toBeNull(); }); + it('should not render a clickable link for javascript: URLs', () => { + const consoleError = jest + .spyOn(console, 'error') + .mockImplementation(() => {}); + render(); + 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(); + const link = screen.getByText('/posts/1') as HTMLAnchorElement; + expect(link.getAttribute('href')).toEqual('/posts/1'); + }); + it('should be customized by a theme', async () => { render(); expect(screen.getByTestId('themed')).toBeDefined(); diff --git a/packages/ra-ui-materialui/src/field/UrlField.stories.tsx b/packages/ra-ui-materialui/src/field/UrlField.stories.tsx index 155b70761af..1379aabecb3 100644 --- a/packages/ra-ui-materialui/src/field/UrlField.stories.tsx +++ b/packages/ra-ui-materialui/src/field/UrlField.stories.tsx @@ -72,3 +72,16 @@ export const Themed = () => { ); }; + +export const UnsafeUrl = () => ( + + + + + + + +); diff --git a/packages/ra-ui-materialui/src/field/UrlField.tsx b/packages/ra-ui-materialui/src/field/UrlField.tsx index 84bd0e86327..ff5aad2655b 100644 --- a/packages/ra-ui-materialui/src/field/UrlField.tsx +++ b/packages/ra-ui-materialui/src/field/UrlField.tsx @@ -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'; @@ -41,7 +41,7 @@ const UrlFieldImpl = < return (