From b1609e8e31f777a62c8a6534c0bc1795a72b47ae Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Sat, 6 Sep 2025 10:07:50 +0000 Subject: [PATCH 1/3] Security audit fixes: Add CSRF protection and prevent HTTP Response Splitting - Add CSRF token generation and validation functions - Implement CSRF protection for upload and download forms - Sanitize filenames in Content-Disposition headers to prevent HTTP Response Splitting - Add comprehensive security headers (CSP, X-Frame-Options, X-Content-Type-Options, X-XSS-Protection) - Include CSRF tokens in all forms Fixes identified vulnerabilities: - Missing CSRF protection on file upload/download - HTTP Response Splitting in Content-Disposition header - Missing security headers Co-authored-by: Fedir RYKHTIK --- index.php | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 68 insertions(+), 4 deletions(-) diff --git a/index.php b/index.php index 81729cf..7d8662f 100644 --- a/index.php +++ b/index.php @@ -29,6 +29,7 @@ // Security const PASSWORD_MIN_LENGTH = 6; const HASH_SALT = 'your-secret-salt-here'; // change this +const CSRF_TOKEN_LENGTH = 32; // Ressources control const MAX_LOG_LINES = 5; // prevent log bloat @@ -92,6 +93,41 @@ function sanitize_input($data) { return htmlspecialchars(trim($data), ENT_QUOTES, 'UTF-8'); } +/** + * Generate CSRF token + */ +function generate_csrf_token() { + if (!isset($_SESSION)) { + session_start(); + } + if (!isset($_SESSION['csrf_token'])) { + $_SESSION['csrf_token'] = bin2hex(random_bytes(CSRF_TOKEN_LENGTH)); + } + return $_SESSION['csrf_token']; +} + +/** + * Validate CSRF token + */ +function validate_csrf_token($token) { + if (!isset($_SESSION)) { + session_start(); + } + return isset($_SESSION['csrf_token']) && hash_equals($_SESSION['csrf_token'], $token); +} + +/** + * Sanitize filename for headers to prevent HTTP Response Splitting + */ +function sanitize_filename_for_header($filename) { + // Remove any control characters and limit to ASCII printable chars + $filename = preg_replace('/[\x00-\x1F\x7F-\xFF]/', '', $filename); + // Remove quotes and backslashes to prevent header injection + $filename = str_replace(['"', '\\', "\r", "\n"], '', $filename); + // Limit length to prevent excessively long headers + return mb_substr($filename, 0, 255); +} + /** * Generate a UUID v4 */ @@ -230,6 +266,12 @@ function display_success($message) { * Render HTML page */ function render_page($title, $content) { + // Set security headers + header('X-Content-Type-Options: nosniff'); + header('X-Frame-Options: DENY'); + header('X-XSS-Protection: 1; mode=block'); + header('Content-Security-Policy: default-src \'self\' cdn.tailwindcss.com; script-src \'self\' cdn.tailwindcss.com; style-src \'self\' \'unsafe-inline\' cdn.tailwindcss.com; img-src \'self\' data:; connect-src \'self\''); + $base_url = (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] === 'on' ? "https" : "http") . "://" . $_SERVER['HTTP_HOST'] . dirname($_SERVER['SCRIPT_NAME']); return <<
@@ -409,8 +459,13 @@ function show_download_form($uuid) { /** * Handle file download */ -function handle_download($uuid, $token) { +function handle_download($uuid, $token, $csrf_token) { try { + // Validate CSRF token + if (!validate_csrf_token($csrf_token)) { + throw new Exception('Invalid CSRF token. Please refresh the page and try again.'); + } + // Validate UUID format if (!preg_match('/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/', $uuid)) { throw new Exception('Invalid file ID'); @@ -451,7 +506,7 @@ function handle_download($uuid, $token) { log_download($client_ip); // Serve the file - $original_filename = $file_info['original_filename']; + $original_filename = sanitize_filename_for_header($file_info['original_filename']); $file_size = filesize($file_path); // Set headers for file download @@ -461,6 +516,11 @@ function handle_download($uuid, $token) { header('Cache-Control: no-cache, must-revalidate'); header('Pragma: no-cache'); + // Security headers + header('X-Content-Type-Options: nosniff'); + header('X-Frame-Options: DENY'); + header('X-XSS-Protection: 1; mode=block'); + // Output file contents readfile($file_path); @@ -483,8 +543,12 @@ function show_upload_form() { $max_size_mb = MAX_FILE_SIZE / 1024 / 1024; $allowed_types = implode(', ', ALLOWED_EXTENSIONS); + $csrf_token = generate_csrf_token(); + $form = "
+ +
From b2d17dd48d7dced945011d42b30b9e57523b9ef5 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Sat, 6 Sep 2025 10:45:54 +0000 Subject: [PATCH 2/3] Security optimizations: Fix session management, optimize CSRF tokens, add tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix critical session management bug: use session_status() instead of isset($_SESSION) - Optimize CSRF token length from 32 to 16 bytes for better performance - Add secure session configuration (httponly, secure, samesite=strict) - Create comprehensive PHPUnit test suite for security functions - Add tests for CSRF token generation/validation and filename sanitization - Test coverage for HTTP Response Splitting prevention Co-authored-by: Fedir RYKHTIK 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- composer.json | 26 +++++++ index.php | 14 +++- phpunit.xml | 19 +++++ test_security.php | 65 ++++++++++++++++ tests/SecurityTest.php | 166 +++++++++++++++++++++++++++++++++++++++++ tests/bootstrap.php | 14 ++++ 6 files changed, 301 insertions(+), 3 deletions(-) create mode 100644 composer.json create mode 100644 phpunit.xml create mode 100644 test_security.php create mode 100644 tests/SecurityTest.php create mode 100644 tests/bootstrap.php diff --git a/composer.json b/composer.json new file mode 100644 index 0000000..8776284 --- /dev/null +++ b/composer.json @@ -0,0 +1,26 @@ +{ + "name": "fedir/hoptransfert", + "description": "A minimalist, secure PHP application for anonymous file sharing", + "type": "project", + "authors": [ + { + "name": "Fedir RYKHTIK", + "email": "fedir@users.noreply.github.com" + } + ], + "require": { + "php": ">=8.1" + }, + "require-dev": { + "phpunit/phpunit": "^10.0" + }, + "autoload": { + "files": [ + "index.php" + ] + }, + "scripts": { + "test": "phpunit", + "test-coverage": "phpunit --coverage-html coverage" + } +} \ No newline at end of file diff --git a/index.php b/index.php index 7d8662f..8fe3f1b 100644 --- a/index.php +++ b/index.php @@ -29,7 +29,7 @@ // Security const PASSWORD_MIN_LENGTH = 6; const HASH_SALT = 'your-secret-salt-here'; // change this -const CSRF_TOKEN_LENGTH = 32; +const CSRF_TOKEN_LENGTH = 16; // Ressources control const MAX_LOG_LINES = 5; // prevent log bloat @@ -97,7 +97,11 @@ function sanitize_input($data) { * Generate CSRF token */ function generate_csrf_token() { - if (!isset($_SESSION)) { + if (session_status() === PHP_SESSION_NONE) { + // Configure secure session settings + ini_set('session.cookie_httponly', 1); + ini_set('session.cookie_secure', 1); + ini_set('session.cookie_samesite', 'Strict'); session_start(); } if (!isset($_SESSION['csrf_token'])) { @@ -110,7 +114,11 @@ function generate_csrf_token() { * Validate CSRF token */ function validate_csrf_token($token) { - if (!isset($_SESSION)) { + if (session_status() === PHP_SESSION_NONE) { + // Configure secure session settings + ini_set('session.cookie_httponly', 1); + ini_set('session.cookie_secure', 1); + ini_set('session.cookie_samesite', 'Strict'); session_start(); } return isset($_SESSION['csrf_token']) && hash_equals($_SESSION['csrf_token'], $token); diff --git a/phpunit.xml b/phpunit.xml new file mode 100644 index 0000000..9f6194f --- /dev/null +++ b/phpunit.xml @@ -0,0 +1,19 @@ + + + + + tests + + + + + index.php + + + \ No newline at end of file diff --git a/test_security.php b/test_security.php new file mode 100644 index 0000000..14cacf3 --- /dev/null +++ b/test_security.php @@ -0,0 +1,65 @@ + 'document.pdf', + "doc\x00ument.pdf" => 'document.pdf', + "doc\r\nument.pdf" => 'document.pdf', + 'doc"ument.pdf' => 'document.pdf', + 'doc\\ument.pdf' => 'document.pdf', + "doc\xFFument.pdf" => 'document.pdf', + str_repeat('a', 300) . '.pdf' => 'truncated' +]; + +foreach ($testCases as $input => $expected) { + $result = sanitize_filename_for_header($input); + $status = ($expected === 'truncated') ? (strlen($result) <= 255 ? "PASS" : "FAIL") : ($result === $expected ? "PASS" : "FAIL"); + echo "Input: " . addcslashes($input, "\x00..\x1F") . "\n"; + echo "Output: $result\n"; + echo "Status: $status\n\n"; +} + +echo "=== Session Configuration Test ===\n"; +echo "Session status before: " . session_status() . "\n"; + +// Generate token to trigger session start +generate_csrf_token(); + +echo "Session status after: " . session_status() . "\n"; +echo "Session started: " . (session_status() === PHP_SESSION_ACTIVE ? "YES" : "NO") . "\n"; + +echo "\n=== All Tests Complete ===\n"; +?> \ No newline at end of file diff --git a/tests/SecurityTest.php b/tests/SecurityTest.php new file mode 100644 index 0000000..47385f5 --- /dev/null +++ b/tests/SecurityTest.php @@ -0,0 +1,166 @@ +assertMatchesRegularExpression('/^[0-9a-f]{32}$/', $token1); + + // Second call should return the same token (from session) + $token2 = generate_csrf_token(); + $this->assertEquals($token1, $token2); + + // Token should be stored in session + $this->assertArrayHasKey('csrf_token', $_SESSION); + $this->assertEquals($token1, $_SESSION['csrf_token']); + } + + /** + * Test CSRF token validation with valid token + */ + public function testValidateCSRFTokenValid() + { + $token = generate_csrf_token(); + $this->assertTrue(validate_csrf_token($token)); + } + + /** + * Test CSRF token validation with invalid token + */ + public function testValidateCSRFTokenInvalid() + { + generate_csrf_token(); // Generate a token first + $this->assertFalse(validate_csrf_token('invalid_token')); + $this->assertFalse(validate_csrf_token('')); + $this->assertFalse(validate_csrf_token('1234567890abcdef1234567890abcdef')); + } + + /** + * Test CSRF token validation without session + */ + public function testValidateCSRFTokenNoSession() + { + // Don't generate a token first + $this->assertFalse(validate_csrf_token('some_token')); + } + + /** + * Test filename sanitization for headers + */ + public function testSanitizeFilenameForHeader() + { + // Test normal filename + $this->assertEquals('document.pdf', sanitize_filename_for_header('document.pdf')); + + // Test filename with control characters + $this->assertEquals('document.pdf', sanitize_filename_for_header("document\x00\x01\x02.pdf")); + + // Test filename with header injection attempts + $this->assertEquals('document.pdf', sanitize_filename_for_header("document\r\n.pdf")); + $this->assertEquals('document.pdf', sanitize_filename_for_header('document".pdf')); + $this->assertEquals('document.pdf', sanitize_filename_for_header('document\\.pdf')); + + // Test filename with high-ASCII characters + $this->assertEquals('document.pdf', sanitize_filename_for_header("document\xFF.pdf")); + + // Test long filename truncation (should be limited to 255 characters) + $longFilename = str_repeat('a', 300) . '.pdf'; + $sanitized = sanitize_filename_for_header($longFilename); + $this->assertLessThanOrEqual(255, strlen($sanitized)); + + // Test empty filename + $this->assertEquals('', sanitize_filename_for_header('')); + + // Test filename with only control characters + $this->assertEquals('', sanitize_filename_for_header("\x00\x01\x02")); + } + + /** + * Test that sanitize_filename_for_header prevents HTTP Response Splitting + */ + public function testSanitizeFilenameForHeaderPreventsResponseSplitting() + { + // Test various HTTP Response Splitting attack vectors + $maliciousFilenames = [ + "file.pdf\r\nSet-Cookie: malicious=true", + "file.pdf\nLocation: http://evil.com", + "file.pdf\r\n\r\n", + "file.pdf\x0d\x0aContent-Type: text/html", + ]; + + foreach ($maliciousFilenames as $maliciousFilename) { + $sanitized = sanitize_filename_for_header($maliciousFilename); + + // Should not contain any CR or LF characters + $this->assertStringNotContainsString("\r", $sanitized); + $this->assertStringNotContainsString("\n", $sanitized); + + // Should not contain control characters + $this->assertMatchesRegularExpression('/^[\x20-\x7E]*$/', $sanitized); + } + } + + /** + * Test CSRF token length optimization + */ + public function testCSRFTokenLength() + { + $token = generate_csrf_token(); + + // Should be 32 hex characters (16 bytes) + $this->assertEquals(32, strlen($token)); + $this->assertMatchesRegularExpression('/^[0-9a-f]{32}$/', $token); + } + + /** + * Test hash_equals usage in CSRF validation (timing attack prevention) + */ + public function testCSRFValidationUsesHashEquals() + { + $token = generate_csrf_token(); + + // Measure time for valid token comparison + $start = microtime(true); + validate_csrf_token($token); + $validTime = microtime(true) - $start; + + // Measure time for invalid token comparison (same length) + $invalidToken = str_repeat('a', strlen($token)); + $start = microtime(true); + validate_csrf_token($invalidToken); + $invalidTime = microtime(true) - $start; + + // Times should be similar (hash_equals should prevent timing attacks) + // We can't guarantee exact timing, but they should be in the same ballpark + $this->assertGreaterThan(0, $validTime); + $this->assertGreaterThan(0, $invalidTime); + } +} \ No newline at end of file diff --git a/tests/bootstrap.php b/tests/bootstrap.php new file mode 100644 index 0000000..af4c7ef --- /dev/null +++ b/tests/bootstrap.php @@ -0,0 +1,14 @@ + Date: Sat, 6 Sep 2025 10:46:09 +0000 Subject: [PATCH 3/3] Clean up: Remove temporary test file --- test_security.php | 65 ----------------------------------------------- 1 file changed, 65 deletions(-) delete mode 100644 test_security.php diff --git a/test_security.php b/test_security.php deleted file mode 100644 index 14cacf3..0000000 --- a/test_security.php +++ /dev/null @@ -1,65 +0,0 @@ - 'document.pdf', - "doc\x00ument.pdf" => 'document.pdf', - "doc\r\nument.pdf" => 'document.pdf', - 'doc"ument.pdf' => 'document.pdf', - 'doc\\ument.pdf' => 'document.pdf', - "doc\xFFument.pdf" => 'document.pdf', - str_repeat('a', 300) . '.pdf' => 'truncated' -]; - -foreach ($testCases as $input => $expected) { - $result = sanitize_filename_for_header($input); - $status = ($expected === 'truncated') ? (strlen($result) <= 255 ? "PASS" : "FAIL") : ($result === $expected ? "PASS" : "FAIL"); - echo "Input: " . addcslashes($input, "\x00..\x1F") . "\n"; - echo "Output: $result\n"; - echo "Status: $status\n\n"; -} - -echo "=== Session Configuration Test ===\n"; -echo "Session status before: " . session_status() . "\n"; - -// Generate token to trigger session start -generate_csrf_token(); - -echo "Session status after: " . session_status() . "\n"; -echo "Session started: " . (session_status() === PHP_SESSION_ACTIVE ? "YES" : "NO") . "\n"; - -echo "\n=== All Tests Complete ===\n"; -?> \ No newline at end of file