From 4ca63dceb6117441ae44530ec09fbcc6192164f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 06:24:58 +0000 Subject: [PATCH 1/3] Validate page names on every save and tie upload content type to the extension - Saving an existing page (isNew unset) skipped isValidPageName(), so pages could be created in hidden folders or in the statically served uploads folder. The name is now checked for every save. - Uploads only checked that the detected content type and the extension were each allowed, so e.g. a PDF could be stored as x.png and then be handed to ImageMagick as PDF. The content type must now match the extension, the format is passed to ImageMagick explicitly, and images above the new MAX_IMAGE_PIXELS setting are refused before being decoded. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01HYvivwGowMczvsbn1ciBmR --- config.php | 6 ++++ functions.php | 36 +++++++++++++++++++ index.php | 22 +++++++++--- tests/Integration/ImageProcessingTest.php | 11 ++++++ tests/Integration/ImageTooLargeTest.php | 43 +++++++++++++++++++++++ tests/Integration/PageNameTest.php | 17 +++++++++ 6 files changed, 130 insertions(+), 5 deletions(-) create mode 100644 tests/Integration/ImageTooLargeTest.php diff --git a/config.php b/config.php index febd3c7..a31146c 100644 --- a/config.php +++ b/config.php @@ -118,6 +118,12 @@ // uploaded images with these extensions need to be converted to another format (see also CONVERT_FORMAT) define('IMAGE_EXTS_TO_CONVERT', 'heic,heif'); +// MAX_IMAGE_PIXELS +// +// Images with more pixels (width x height) are not processed (shrunk, rotated or converted) +// and are refused, because decoding huge images can use up all memory of the server. +define('MAX_IMAGE_PIXELS', 100000000); + // CONVERT_FORMAT // format to convert uploaded images to which need to be converted (see IMAGE_EXTS_TO_CONVERT) define('CONVERT_FORMAT', 'jpg'); diff --git a/functions.php b/functions.php index ca0f82e..d9b1ad7 100644 --- a/functions.php +++ b/functions.php @@ -211,6 +211,42 @@ function sanitizeUploadedSvg($tmpName) return $clean; } +/** + * Content types (as detected from the file content) which a file with the given extension may have. + * Without this, the content of any accepted type could be stored (and processed by ImageMagick) under + * any accepted extension, e.g. a PDF as "x.png". Extensions without an entry are not restricted further. + */ +function uploadTypesForExt($ext) +{ + $types = array( + 'bmp' => array('image/bmp', 'image/x-ms-bmp'), + 'gif' => array('image/gif'), + 'heic' => array('image/heic', 'image/heif', 'image/heic-sequence', 'image/heif-sequence'), + 'heif' => array('image/heic', 'image/heif', 'image/heic-sequence', 'image/heif-sequence'), + 'jpg' => array('image/jpeg', 'image/pjpeg'), + 'jpeg' => array('image/jpeg', 'image/pjpeg'), + 'pdf' => array('application/pdf'), + 'png' => array('image/png'), + 'webp' => array('image/webp'), + ); + return $types[$ext] ?? null; +} + +function uploadTypeMatchesExt($type, $ext) +{ + $allowed = uploadTypesForExt($ext); + return $allowed === null || in_array($type, $allowed, true); +} + +/** + * Prefix for the file name given to ImageMagick, which forces the format instead of guessing it from + * the content of the file ("png:/path/file.png") + */ +function imageMagickFormatPrefix($ext) +{ + return ($ext === 'jpg' || $ext === 'jpeg') ? 'jpeg:' : $ext . ':'; +} + function hasValidUploadExt($fileName) { return !isHiddenFile($fileName) && in_array(getFileExt($fileName), validUploadExts(), true); diff --git a/index.php b/index.php index fda8afc..17d3724 100644 --- a/index.php +++ b/index.php @@ -288,12 +288,13 @@ function destroy_session() $page = str_replace(array('|','#'), '', $page); $filename = fileNameForPage($page); } - if ($isNew && (file_exists($filename) || !isValidPageName($page))) + // (the name is checked when saving existing pages too: the client decides whether a page is new) + if (($isNew && file_exists($filename)) || !isValidPageName($page)) { - $msg .= file_exists($filename) + $msg .= ($isNew && file_exists($filename)) ? sprintf(__("Error creating page '%s' - it already exists! Please choose a different name, or %s the existing page (this discards current text!)!"), h($page), "".__('edit')."")."\n" : sprintf(__("Error creating page '%s' - invalid page name! Page names must not start with '%s/', or contain empty or hidden ('.'-prefixed) folder names."), h($page), h(UPLOAD_FOLDER))."\n"; - $action = 'new'; + $action = $isNew ? 'new' : 'edit'; $text = $newText; $newPage = $page; if (GIT_COMMIT_ENABLED) @@ -571,7 +572,8 @@ function destroy_session() $typeAllowed = ($svgData !== false); } } - if ($typeAllowed && hasValidUploadExt($dstName)) + // (and the content must be of the type belonging to the extension, which decides how it is processed) + if ($typeAllowed && hasValidUploadExt($dstName) && ($fileExt === 'svg' || uploadTypeMatchesExt($fileType, $fileExt))) { $path = PAGES_PATH . "/". UPLOAD_FOLDER . "/$dstName"; $doResize = isset($_POST['resize']) && $_POST['resize'] === 'true'; @@ -597,7 +599,17 @@ function destroy_session() { try { - $img = new Imagick($path); + // the format is given explicitly, ImageMagick must not guess it from the content + $source = imageMagickFormatPrefix($fileExt) . $path; + $probe = new Imagick(); + $probe->pingImage($source); + $pixels = $probe->getImageWidth() * $probe->getImageHeight(); + $probe->clear(); + if ($pixels > MAX_IMAGE_PIXELS) + { + throw new ImagickException('image has too many pixels'); + } + $img = new Imagick($source); if ($doResize) { $size = array($img->getImageWidth(), $img->getImageHeight()); diff --git a/tests/Integration/ImageProcessingTest.php b/tests/Integration/ImageProcessingTest.php index 96cd1de..273e2f4 100644 --- a/tests/Integration/ImageProcessingTest.php +++ b/tests/Integration/ImageProcessingTest.php @@ -48,6 +48,17 @@ private function uploadResized(string $name, string $content, array $extra = []) return $this->noteAfter($this->upload($name, $content, 'image/png', ['resize' => 'true'] + $extra)); } + // --- type and extension ------------------------------------------------------------ + + public function testContentOfAnotherAcceptedTypeIsNotStoredUnderAnImageExtension(): void + { + // a PDF as "x.png" would be handed to ImageMagick as PDF (Ghostscript) when the format is guessed from the content + $pdf = "%PDF-1.4\n1 0 obj<>endobj\ntrailer<>\n%%EOF\n"; + $note = $this->uploadResized('x.png', $pdf); + $this->assertStringContainsString('invalid file type', $note); + $this->assertSame([], $this->uploadedFiles()); + } + // --- resize ---------------------------------------------------------------------- public function testLargeLandscapeImageIsShrunkKeepingTheAspectRatio(): void diff --git a/tests/Integration/ImageTooLargeTest.php b/tests/Integration/ImageTooLargeTest.php new file mode 100644 index 0000000..60bb495 --- /dev/null +++ b/tests/Integration/ImageTooLargeTest.php @@ -0,0 +1,43 @@ + 100]; + } + + protected function setUp(): void + { + if (!extension_loaded('imagick')) { + $this->markTestSkipped('The Imagick extension is not installed'); + } + parent::setUp(); + } + + private static function image(int $width, int $height): string + { + $img = new \Imagick(); + $img->newImage($width, $height, new \ImagickPixel('red'), 'png'); + return $img->getImagesBlob(); + } + + public function testImageWithTooManyPixelsIsRefusedAndRemoved(): void + { + $note = $this->noteAfter($this->upload('big.png', self::image(11, 10), 'image/png', ['resize' => 'true'])); + $this->assertStringContainsString('could not be processed', $note); + $this->assertSame([], $this->uploadedFiles()); + } + + public function testImageWithinTheLimitIsAccepted(): void + { + $note = $this->noteAfter($this->upload('ok.png', self::image(10, 10), 'image/png', ['resize' => 'true'])); + $this->assertStringContainsString('uploaded', $note); + $this->assertSame(['ok.png'], $this->uploadedFiles()); + } +} diff --git a/tests/Integration/PageNameTest.php b/tests/Integration/PageNameTest.php index 28743db..a086ac4 100644 --- a/tests/Integration/PageNameTest.php +++ b/tests/Integration/PageNameTest.php @@ -79,6 +79,23 @@ public function testInvalidPageNamesAreRefused(string $name): void $this->assertSame($before, $this->server->pageFiles()); } + #[DataProvider('invalidNames')] + public function testInvalidPageNamesAreRefusedWhenSavingWithoutTheNewFlag(string $name): void + { + // the client decides whether a page is new, so the name has to be checked either way + $before = $this->server->pageFiles(); + $note = $this->noteInResponse($this->savePage($name, 'text', false)); + $this->assertStringContainsString('invalid page name', $note); + $this->assertSame($before, $this->server->pageFiles()); + $this->assertFileDoesNotExist($this->server->pagesDir() . '/.hidden/evil.md'); + } + + public function testExistingPagesCanStillBeEdited(): void + { + $this->assertSame(303, $this->savePage('Home', 'edited', false)->status); + $this->assertSame('edited', $this->pageText('Home')); + } + public function testExistingPagesAreNotOverwrittenWhenCreatingPages(): void { $note = $this->noteInResponse($this->savePage('Home', 'overwritten')); From 0076d9cb2391a4635966889285668fa6b27f607a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 06:54:07 +0000 Subject: [PATCH 2/3] Add security headers and deny access to files visitors do not need - index.php and api.php now send a Content-Security-Policy (own scripts and styles only, nonce for the upload page script, hashes for the wiki's two inline handlers, no framing/plugins/base, forms to self), nosniff, X-Frame-Options, Referrer-Policy, Permissions-Policy, COOP and, over HTTPS, Strict-Transport-Security. - Apache: only index.php and api.php are reachable among the PHP scripts; composer/phpunit files, markdown docs and the license are denied; the tests, locales and Michelf folders get their own .htaccess denying access. - nginx: equivalent rules in INSTALL.md and the tested template; static files get nosniff. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01HYvivwGowMczvsbn1ciBmR --- .htaccess | 12 +++++ INSTALL.md | 21 ++++++++ Michelf/.htaccess | 2 + auth.php | 5 ++ auth_functions.php | 54 +++++++++++++++++++ index.php | 10 ++-- locales/.htaccess | 2 + tests/.htaccess | 2 + tests/Integration/IpAllowlistDeniedTest.php | 7 +++ tests/Integration/SecurityHeadersTest.php | 57 +++++++++++++++++++++ tests/Server/ServerConfigTest.php | 25 +++++++++ tests/Server/nginx/w2.conf.template | 14 +++++ tests/Unit/AuthFunctionsTest.php | 16 ++++++ 13 files changed, 222 insertions(+), 5 deletions(-) create mode 100644 Michelf/.htaccess create mode 100644 locales/.htaccess create mode 100644 tests/.htaccess create mode 100644 tests/Integration/SecurityHeadersTest.php diff --git a/.htaccess b/.htaccess index 6c5a257..ccad1f7 100644 --- a/.htaccess +++ b/.htaccess @@ -1,6 +1,17 @@ # Deny access to hidden files and folders such as .git (except .well-known) RedirectMatch 404 /\.(?!well-known/) +# Only index.php and api.php are entry points; the other scripts are included by them and must not be +# reachable (see also the .htaccess files in the folders which only have such files or nothing public) + + Require all denied + + +# Files which are of no use for visitors: dependency and test configuration, documentation, license + + Require all denied + + # Don't list the contents of folders (like the uploads) without index file Options -Indexes @@ -9,5 +20,6 @@ Options -Indexes Header set Cache-Control "max-age=31536000, immutable" + Header set X-Content-Type-Options "nosniff" diff --git a/INSTALL.md b/INSTALL.md index 859d0a3..1b765d6 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -119,8 +119,22 @@ W2 location prefix if W2 is not installed in the web root): location ~ /\.(?!well-known/) { deny all; } location ^~ /vendor/ { deny all; } location ^~ /pages/ { deny all; } +location ^~ /tests/ { deny all; } +location ^~ /locales/ { deny all; } +location ^~ /Michelf/ { deny all; } +location ^~ /config.php { deny all; } +location ^~ /functions.php { deny all; } +location ^~ /auth.php { deny all; } +location ^~ /auth_functions.php { deny all; } +location ^~ /composer. { deny all; } +location ^~ /phpunit.xml { deny all; } +location ^~ /LICENSE { deny all; } +location ^~ /README.md { deny all; } +location ^~ /INSTALL.md { deny all; } +location ^~ /CLAUDE.md { deny all; } location ~* \.(js|css|svg|png)$ { add_header Cache-Control "max-age=31536000, immutable"; + add_header X-Content-Type-Options nosniff; } location ^~ /images/ { location ~* \.(php[0-9]?|pht|phtml|phar)$ { deny all; } @@ -134,6 +148,13 @@ location ^~ /images/ { } ``` +The scripts of the wiki send security headers with every response (Content-Security-Policy, which only allows +scripts and styles from the wiki itself and forbids framing, `X-Content-Type-Options`, `X-Frame-Options`, +`Referrer-Policy`, `Permissions-Policy`, and `Strict-Transport-Security` for requests over HTTPS). Don't add +headers of the same names in the web server, they would apply to the same responses twice. The rules above +additionally deny access to files which are not needed by visitors (the included scripts other than `index.php` +and `api.php`, the `tests`, `locales` and `Michelf` folders, `composer.json`, the documentation and so on). + ### Upload size limits Uploads are limited by PHP: `upload_max_filesize` (the largest single file) and diff --git a/Michelf/.htaccess b/Michelf/.htaccess new file mode 100644 index 0000000..2a97d2d --- /dev/null +++ b/Michelf/.htaccess @@ -0,0 +1,2 @@ +# The Markdown library is loaded by index.php, never requested directly. +Require all denied diff --git a/auth.php b/auth.php index 0ef6fbf..8d4b1c8 100644 --- a/auth.php +++ b/auth.php @@ -17,6 +17,11 @@ session_name(W2_SESSION_NAME); session_start(); +foreach ( securityHeaders($_SERVER) as $name => $value ) +{ + header("$name: $value"); +} + // token protecting state-changing requests against cross-site request forgery if ( empty($_SESSION['csrf_token']) ) { diff --git a/auth_functions.php b/auth_functions.php index b22a5e7..d1996bd 100644 --- a/auth_functions.php +++ b/auth_functions.php @@ -6,6 +6,10 @@ * Nothing happens when this file is loaded, see auth.php for that. */ +// Inline event handlers used by the wiki's own pages (allowed in the Content-Security-Policy by their hash) +const HANDLER_TOGGLE_DRAWER = 'toggleDrawer(); return false;'; +const HANDLER_GO_BACK = 'history.go(-1);'; + function csrfToken() { return $_SESSION['csrf_token']; @@ -117,3 +121,53 @@ function csrfField() { return ""; } + +/** + * Nonce which allows the inline script of a page in the Content-Security-Policy (new for every request) + */ +function cspNonce() +{ + static $nonce = null; + return $nonce ??= base64_encode(random_bytes(16)); +} + +/** + * The Content-Security-Policy of the pages of the wiki: only scripts and styles from the wiki itself + * (plus the nonce'd inline script, and the wiki's own inline event handlers by hash), no plugins, no + * framing, forms only to the wiki. Images may come from anywhere, as pages can include external images. + */ +function contentSecurityPolicy() +{ + $handlerHashes = array_map(fn($handler) => "'sha256-" . base64_encode(hash('sha256', $handler, true)) . "'", + array(HANDLER_TOGGLE_DRAWER, HANDLER_GO_BACK)); + return implode('; ', array( + "default-src 'self'", + "script-src 'self' 'nonce-" . cspNonce() . "' 'unsafe-hashes' " . implode(' ', $handlerHashes), + "style-src 'self'", + "img-src 'self' data: http: https:", + "object-src 'none'", + "base-uri 'none'", + "form-action 'self'", + "frame-ancestors 'none'" + )); +} + +/** + * Security headers sent with every response of the wiki's scripts ($server is $_SERVER) + */ +function securityHeaders(array $server) +{ + $headers = array( + 'Content-Security-Policy' => contentSecurityPolicy(), + 'X-Content-Type-Options' => 'nosniff', + 'X-Frame-Options' => 'DENY', + 'Referrer-Policy' => 'same-origin', + 'Permissions-Policy' => 'camera=(), microphone=(), geolocation=(), payment=(), usb=()', + 'Cross-Origin-Opener-Policy' => 'same-origin' + ); + if ( isHttpsRequest($server) ) + { + $headers['Strict-Transport-Security'] = 'max-age=31536000'; + } + return $headers; +} diff --git a/index.php b/index.php index 17d3724..3cc116e 100644 --- a/index.php +++ b/index.php @@ -57,7 +57,7 @@ function printFooter() function printDrawer() { print "
\n". - " \"".__('Close')."\"\n". + " \"".__('Close')."\"\n". "
".__('Markdown Syntax Helper')."
\n". "
\n". "# ".__('Header')." 1
". @@ -90,7 +90,7 @@ function printDrawer() "--- ".__('Horizontal rule')."
\n". "
\n". "
\n". - " \n". + " \n". " \n". " \"".__('Formatting\n". " \"".__('Formatting\n". @@ -380,7 +380,7 @@ function destroy_session() $html .= "

\n"; $html .= "\n"; $html .= ''."\n"; - $html .= ''."\n"; + $html .= ''."\n"; $html .= "

\n"; } else if ( $action === 'logout' ) @@ -414,7 +414,7 @@ function destroy_session() ''. ''. "\n

\n"; - $html .= '