🔒(frontend) enhance file download security (#889)
## Purpose 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.
This commit is contained in:
@@ -8,6 +8,10 @@ and this project adheres to
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
## Fixed
|
||||
|
||||
- 🔒(frontend) enhance file download security #889
|
||||
|
||||
## Added
|
||||
|
||||
- 🚸(backend) make document search on title accent-insensitive #874
|
||||
|
||||
@@ -15,6 +15,7 @@ import { useCallback, useMemo } from 'react';
|
||||
import { RiDownload2Fill } from 'react-icons/ri';
|
||||
|
||||
import { downloadFile, exportResolveFileUrl } from '@/docs/doc-export';
|
||||
import { isSafeUrl } from '@/utils/url';
|
||||
|
||||
export const FileDownloadButton = ({
|
||||
open,
|
||||
@@ -59,7 +60,11 @@ export const FileDownloadButton = ({
|
||||
*/
|
||||
if (!url.includes(window.location.hostname) && !url.includes('base64')) {
|
||||
if (!editor.resolveFileUrl) {
|
||||
window.open(url);
|
||||
if (!isSafeUrl(url)) {
|
||||
return;
|
||||
}
|
||||
|
||||
window.open(url, '_blank', 'noopener,noreferrer');
|
||||
} else {
|
||||
void editor
|
||||
.resolveFileUrl(url)
|
||||
|
||||
110
src/frontend/apps/impress/src/utils/__tests__/url.test.tsx
Normal file
110
src/frontend/apps/impress/src/utils/__tests__/url.test.tsx
Normal file
@@ -0,0 +1,110 @@
|
||||
import { isSafeUrl } from '@/utils/url';
|
||||
|
||||
describe('isSafeUrl', () => {
|
||||
// XSS Attacks
|
||||
const xssUrls = [
|
||||
"javascript:alert('xss')",
|
||||
"data:text/html,<script>alert('xss')</script>",
|
||||
"vbscript:msgbox('xss')",
|
||||
"expression(alert('xss'))",
|
||||
"https://example.com/\"><script>alert('xss')</script>",
|
||||
"https://example.com/\"><img src=x onerror=alert('xss')>",
|
||||
"javascript:/*--></title></style></textarea></script><xmp><svg/onload='+/\"/+/onmouseover=1/+/[*/[]/+alert(1)//'>",
|
||||
];
|
||||
|
||||
// Directory Traversal
|
||||
const traversalUrls = [
|
||||
'https://example.com/../../etc/passwd',
|
||||
'https://example.com/..%2F..%2Fetc%2Fpasswd',
|
||||
'https://example.com/..\\..\\Windows\\System32\\config\\SAM',
|
||||
];
|
||||
|
||||
// SQL Injection
|
||||
const sqlInjectionUrls = [
|
||||
"https://example.com/' OR '1'='1",
|
||||
'https://example.com/; DROP TABLE users;',
|
||||
"https://example.com/' OR 1=1 --",
|
||||
];
|
||||
|
||||
// Malicious Encodings
|
||||
const encodingUrls = [
|
||||
"https://example.com/%3Cscript%3Ealert('xss')%3C/script%3E",
|
||||
'https://example.com/%00',
|
||||
'https://example.com/\\0',
|
||||
'https://example.com/file.php%00.jpg',
|
||||
];
|
||||
|
||||
// Unauthorized Protocols
|
||||
const protocolUrls = [
|
||||
'file:///etc/passwd',
|
||||
'ftp://attacker.com/malware.exe',
|
||||
'telnet://attacker.com',
|
||||
];
|
||||
|
||||
// Long URLs
|
||||
const longUrls = ['https://example.com/' + 'a'.repeat(2001)];
|
||||
|
||||
// Safe URLs
|
||||
const safeUrls = [
|
||||
'https://example.com',
|
||||
'https://example.com/path/to/file',
|
||||
'https://example.com?param=value',
|
||||
'https://example.com#section',
|
||||
];
|
||||
|
||||
describe('should block XSS attacks', () => {
|
||||
xssUrls.forEach((url) => {
|
||||
it(`should block ${url}`, () => {
|
||||
expect(isSafeUrl(url)).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('should block directory traversal', () => {
|
||||
traversalUrls.forEach((url) => {
|
||||
it(`should block ${url}`, () => {
|
||||
expect(isSafeUrl(url)).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('should block SQL injection', () => {
|
||||
sqlInjectionUrls.forEach((url) => {
|
||||
it(`should block ${url}`, () => {
|
||||
expect(isSafeUrl(url)).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('should block malicious encodings', () => {
|
||||
encodingUrls.forEach((url) => {
|
||||
it(`should block ${url}`, () => {
|
||||
expect(isSafeUrl(url)).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('should block unauthorized protocols', () => {
|
||||
protocolUrls.forEach((url) => {
|
||||
it(`should block ${url}`, () => {
|
||||
expect(isSafeUrl(url)).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('should block long URLs', () => {
|
||||
longUrls.forEach((url) => {
|
||||
it(`should block ${url}`, () => {
|
||||
expect(isSafeUrl(url)).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('should allow safe URLs', () => {
|
||||
safeUrls.forEach((url) => {
|
||||
it(`should allow ${url}`, () => {
|
||||
expect(isSafeUrl(url)).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
51
src/frontend/apps/impress/src/utils/url.ts
Normal file
51
src/frontend/apps/impress/src/utils/url.ts
Normal file
@@ -0,0 +1,51 @@
|
||||
export function isSafeUrl(url: string): boolean {
|
||||
try {
|
||||
// Parse the URL with a base to support relative URLs
|
||||
const parsed = new URL(url, window.location.origin);
|
||||
|
||||
// List of allowed protocols
|
||||
const allowedProtocols = ['http:', 'https:'];
|
||||
|
||||
// Check protocol
|
||||
if (!allowedProtocols.includes(parsed.protocol)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check for dangerous characters in the pathname
|
||||
const dangerousChars = ['<', '>', '"', "'", '(', ')', ';', '=', '{', '}'];
|
||||
if (dangerousChars.some((char) => parsed.pathname.includes(char))) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check URL length (protection against buffer overflow attacks)
|
||||
if (url.length > 2000) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check for malicious encodings
|
||||
if (url.includes('%00') || url.includes('\\0')) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check for XSS injection attempts
|
||||
const xssPatterns = [
|
||||
'<script',
|
||||
'javascript:',
|
||||
'data:',
|
||||
'vbscript:',
|
||||
'expression(',
|
||||
];
|
||||
if (xssPatterns.some((pattern) => url.toLowerCase().includes(pattern))) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check for directory traversal attempts
|
||||
if (url.includes('..') || url.includes('../') || url.includes('..\\')) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user