Skip to content

🔒(frontend) enhance file download security #889

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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 CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
110 changes: 110 additions & 0 deletions src/frontend/apps/impress/src/utils/__tests__/url.test.tsx
Original file line number Diff line number Diff line change
@@ -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 changes: 51 additions & 0 deletions src/frontend/apps/impress/src/utils/url.ts
Original file line number Diff line number Diff line change
@@ -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;
}
}
Loading