Skip to content

Commit 42c0f11

Browse files
🔒️(frontend) enhance file download security
Added a safety check for URLs in the FileDownloadButton component. Now, before opening a URL, it verifies if the URL is safe using the isSafeUrl function. This prevents potentially unsafe URLs from being opened in a new tab.
1 parent 419079a commit 42c0f11

File tree

4 files changed

+171
-1
lines changed

4 files changed

+171
-1
lines changed

CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ and this project adheres to
88

99
## [Unreleased]
1010

11+
## Fixed
12+
13+
- 🔒(frontend) enhance file download security #889
14+
1115
## Added
1216

1317
- 🚸(backend) make document search on title accent-insensitive #874

src/frontend/apps/impress/src/features/docs/doc-editor/components/BlockNoteToolBar/FileDownloadButton.tsx

+6-1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { useCallback, useMemo } from 'react';
1515
import { RiDownload2Fill } from 'react-icons/ri';
1616

1717
import { downloadFile, exportResolveFileUrl } from '@/docs/doc-export';
18+
import { isSafeUrl } from '@/utils/url';
1819

1920
export const FileDownloadButton = ({
2021
open,
@@ -59,7 +60,11 @@ export const FileDownloadButton = ({
5960
*/
6061
if (!url.includes(window.location.hostname) && !url.includes('base64')) {
6162
if (!editor.resolveFileUrl) {
62-
window.open(url);
63+
if (!isSafeUrl(url)) {
64+
return;
65+
}
66+
67+
window.open(url, '_blank', 'noopener,noreferrer');
6368
} else {
6469
void editor
6570
.resolveFileUrl(url)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
import { isSafeUrl } from '@/utils/url';
2+
3+
describe('isSafeUrl', () => {
4+
// XSS Attacks
5+
const xssUrls = [
6+
"javascript:alert('xss')",
7+
"data:text/html,<script>alert('xss')</script>",
8+
"vbscript:msgbox('xss')",
9+
"expression(alert('xss'))",
10+
"https://example.com/\"><script>alert('xss')</script>",
11+
"https://example.com/\"><img src=x onerror=alert('xss')>",
12+
"javascript:/*--></title></style></textarea></script><xmp><svg/onload='+/\"/+/onmouseover=1/+/[*/[]/+alert(1)//'>",
13+
];
14+
15+
// Directory Traversal
16+
const traversalUrls = [
17+
'https://example.com/../../etc/passwd',
18+
'https://example.com/..%2F..%2Fetc%2Fpasswd',
19+
'https://example.com/..\\..\\Windows\\System32\\config\\SAM',
20+
];
21+
22+
// SQL Injection
23+
const sqlInjectionUrls = [
24+
"https://example.com/' OR '1'='1",
25+
'https://example.com/; DROP TABLE users;',
26+
"https://example.com/' OR 1=1 --",
27+
];
28+
29+
// Malicious Encodings
30+
const encodingUrls = [
31+
"https://example.com/%3Cscript%3Ealert('xss')%3C/script%3E",
32+
'https://example.com/%00',
33+
'https://example.com/\\0',
34+
'https://example.com/file.php%00.jpg',
35+
];
36+
37+
// Unauthorized Protocols
38+
const protocolUrls = [
39+
'file:///etc/passwd',
40+
'ftp://attacker.com/malware.exe',
41+
'telnet://attacker.com',
42+
];
43+
44+
// Long URLs
45+
const longUrls = ['https://example.com/' + 'a'.repeat(2001)];
46+
47+
// Safe URLs
48+
const safeUrls = [
49+
'https://example.com',
50+
'https://example.com/path/to/file',
51+
'https://example.com?param=value',
52+
'https://example.com#section',
53+
];
54+
55+
describe('should block XSS attacks', () => {
56+
xssUrls.forEach((url) => {
57+
it(`should block ${url}`, () => {
58+
expect(isSafeUrl(url)).toBe(false);
59+
});
60+
});
61+
});
62+
63+
describe('should block directory traversal', () => {
64+
traversalUrls.forEach((url) => {
65+
it(`should block ${url}`, () => {
66+
expect(isSafeUrl(url)).toBe(false);
67+
});
68+
});
69+
});
70+
71+
describe('should block SQL injection', () => {
72+
sqlInjectionUrls.forEach((url) => {
73+
it(`should block ${url}`, () => {
74+
expect(isSafeUrl(url)).toBe(false);
75+
});
76+
});
77+
});
78+
79+
describe('should block malicious encodings', () => {
80+
encodingUrls.forEach((url) => {
81+
it(`should block ${url}`, () => {
82+
expect(isSafeUrl(url)).toBe(false);
83+
});
84+
});
85+
});
86+
87+
describe('should block unauthorized protocols', () => {
88+
protocolUrls.forEach((url) => {
89+
it(`should block ${url}`, () => {
90+
expect(isSafeUrl(url)).toBe(false);
91+
});
92+
});
93+
});
94+
95+
describe('should block long URLs', () => {
96+
longUrls.forEach((url) => {
97+
it(`should block ${url}`, () => {
98+
expect(isSafeUrl(url)).toBe(false);
99+
});
100+
});
101+
});
102+
103+
describe('should allow safe URLs', () => {
104+
safeUrls.forEach((url) => {
105+
it(`should allow ${url}`, () => {
106+
expect(isSafeUrl(url)).toBe(true);
107+
});
108+
});
109+
});
110+
});
+51
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
export function isSafeUrl(url: string): boolean {
2+
try {
3+
// Parse the URL with a base to support relative URLs
4+
const parsed = new URL(url, window.location.origin);
5+
6+
// List of allowed protocols
7+
const allowedProtocols = ['http:', 'https:'];
8+
9+
// Check protocol
10+
if (!allowedProtocols.includes(parsed.protocol)) {
11+
return false;
12+
}
13+
14+
// Check for dangerous characters in the pathname
15+
const dangerousChars = ['<', '>', '"', "'", '(', ')', ';', '=', '{', '}'];
16+
if (dangerousChars.some((char) => parsed.pathname.includes(char))) {
17+
return false;
18+
}
19+
20+
// Check URL length (protection against buffer overflow attacks)
21+
if (url.length > 2000) {
22+
return false;
23+
}
24+
25+
// Check for malicious encodings
26+
if (url.includes('%00') || url.includes('\\0')) {
27+
return false;
28+
}
29+
30+
// Check for XSS injection attempts
31+
const xssPatterns = [
32+
'<script',
33+
'javascript:',
34+
'data:',
35+
'vbscript:',
36+
'expression(',
37+
];
38+
if (xssPatterns.some((pattern) => url.toLowerCase().includes(pattern))) {
39+
return false;
40+
}
41+
42+
// Check for directory traversal attempts
43+
if (url.includes('..') || url.includes('../') || url.includes('..\\')) {
44+
return false;
45+
}
46+
47+
return true;
48+
} catch {
49+
return false;
50+
}
51+
}

0 commit comments

Comments
 (0)