diff --git a/TODO.md b/TODO.md new file mode 100644 index 0000000..c6874bc --- /dev/null +++ b/TODO.md @@ -0,0 +1,55 @@ +# CodePress CMS - Verbeteringen TODO + +## Kritiek + +- [x] **Path traversal fix** - `str_replace('../')` in `getPage()` is te omzeilen. Gebruik `realpath()` met prefix-check (`CodePressCMS.php:313`) +- [x] **JWT secret fallback** - Standaard `'your-secret-key-change-in-production'` maakt tokens forgeable (`admin-console/config/app.php:11`) +- [x] **executePhpFile() onveilig** - Open `include` wrapper zonder pad-restrictie (`CMSAPI.php:164`) +- [ ] **Plugin auto-loading** - Elke map in `plugins/` wordt blind geladen zonder allowlist of validatie (`PluginManager.php:40`) + +## Hoog + +- [x] **IP spoofing** - `X-Forwarded-For` header wordt blind vertrouwd in MQTTTracker (`MQTTTracker.php:211`) +- [x] **Debug hardcoded** - `'debug' => true` hardcoded in admin config (`admin-console/config/app.php:6`) +- [x] **Cookie security** - Cookies zonder `Secure`/`HttpOnly`/`SameSite` flags (`MQTTTracker.php:70`) +- [ ] **autoLinkPageTitles()** - Regex kan geneste `` tags produceren (`CodePressCMS.php:587`) +- [ ] **extract($data)** - Kan lokale variabelen overschrijven in AuthController (`AuthController.php:77`) +- [ ] **MQTT wachtwoord** - Credentials in plain text JSON (`MQTTTracker.php:37`) + +## Medium + +- [x] **Dead code** - Dubbele `is_dir()` check, tweede blok onbereikbaar (`CodePressCMS.php:328-333`) +- [x] **htmlspecialchars() op bestandspad** - Corrumpeert bestandslookups in `getPage()` en `getContentType()` (`CodePressCMS.php:311, 1294`) +- [x] **Ongebruikte methode** - `scanForPageNames()` wordt nergens aangeroepen (`CodePressCMS.php:658-679`) +- [x] **Orphaned docblock** - Dubbel docblock zonder bijbehorende methode (`CodePressCMS.php:607-611`) +- [x] **Extra ``** - Sluit een tag die nooit geopend is in `getDirectoryListing()` (`CodePressCMS.php:996`) +- [x] **Dubbele require_once** - PluginManager/CMSAPI geladen in zowel index.php als constructor (`CodePressCMS.php:49-50`) +- [x] **require_once autoload** - Autoloader opnieuw geladen in `parseMarkdown()` (`CodePressCMS.php:513`) +- [x] **Breadcrumb titels ongeescaped** - `$title` direct in HTML zonder `htmlspecialchars()` (`CodePressCMS.php:1197`) +- [x] **Zoekresultaat-URLs missen `&lang=`** - Taalparameter ontbreekt (`CodePressCMS.php:264`) +- [x] **Operator precedence bug** - `!$x ?? true` evalueert als `(!$x) ?? true` (`MQTTTracker.php:131`) +- [ ] **Taalwisselaar verliest pagina** - Wisselen van taal navigeert altijd naar homepage (`header.mustache:22`) +- [ ] **ctime is geen creatietijd op Linux** - `stat()` ctime is inode-wijzigingstijd (`CodePressCMS.php:400`) +- [ ] **getGuidePage() dupliceert markdown parsing** - Zelfde CommonMark setup als `parseMarkdown()` (`CodePressCMS.php:854`) +- [ ] **HTMLBlock ontbrekende ``** - Niet-gesloten tags bij null-check (`HTMLBlock.php:68`) +- [ ] **CSRF-bescherming** - Login form zonder CSRF token (`AuthController.php:18`) +- [ ] **formatDisplayName() redundante logica** - Dubbele checks en overtollige str_replace (`CodePressCMS.php:688`) + +## Laag + +- [x] **Hardcoded 'Ga naar'** - Niet vertaalbaar in `autoLinkPageTitles()` (`CodePressCMS.php:587`) +- [x] **HTML lang attribuut** - `` hardcoded i.p.v. dynamisch (`layout.mustache:2`) +- [x] **console.log in productie** - Debug log in app.js (`app.js:54`) +- [x] **Event listener leak** - N globale click listeners in forEach loop (`app.js:85`) +- [x] **Sidebar toggle aria** - Ontbrekende `aria-label` en `aria-expanded` (`CodePressCMS.php:1171`) +- [x] **Taalprefix hardcoded** - Alleen `nl|en` i.p.v. dynamisch uit config (`CodePressCMS.php:691, 190`) +- [ ] **Geen type hints** - Ontbrekende type declarations op properties en methoden +- [ ] **Public properties** - `$config`, `$currentLanguage`, `$searchResults` zouden private moeten zijn +- [ ] **Inline CSS** - ~250 regels statische CSS in template i.p.v. extern bestand +- [ ] **style.css is Bootstrap** - Bestandsnaam is misleidend, Bootstrap wordt mogelijk dubbel geladen +- [ ] **Geen error handling op file_get_contents()** - Meerdere calls zonder return-check +- [ ] **Logger slikt fouten** - `@file_put_contents()` met error suppression +- [ ] **Logger tail() leest heel bestand** - Geheugenprobleem bij grote logbestanden +- [ ] **Externe links missen rel="noreferrer"** +- [ ] **Zoekformulier mist aria-label** +- [ ] **mobile.css override Bootstrap utilities** met `!important` diff --git a/admin-console/config/app.php b/admin-console/config/app.php index 6cea29a..9aa6c2a 100644 --- a/admin-console/config/app.php +++ b/admin-console/config/app.php @@ -3,12 +3,12 @@ return [ 'name' => 'CodePress Admin Console', 'version' => '1.0.0', - 'debug' => true, + 'debug' => $_ENV['APP_DEBUG'] ?? false, 'timezone' => 'Europe/Amsterdam', // Security 'security' => [ - 'jwt_secret' => $_ENV['JWT_SECRET'] ?? 'your-secret-key-change-in-production', + 'jwt_secret' => $_ENV['JWT_SECRET'] ?? throw new \RuntimeException('JWT_SECRET environment variable must be set'), 'jwt_expiration' => 3600, // 1 hour 'session_timeout' => 1800, // 30 minutes 'max_login_attempts' => 5, diff --git a/engine/core/class/CodePressCMS.php b/engine/core/class/CodePressCMS.php index c448faf..785ccf9 100644 --- a/engine/core/class/CodePressCMS.php +++ b/engine/core/class/CodePressCMS.php @@ -45,9 +45,7 @@ class CodePressCMS { $this->currentLanguage = $this->getCurrentLanguage(); $this->translations = $this->loadTranslations($this->currentLanguage); - // Initialize plugin manager - require_once __DIR__ . '/../plugin/PluginManager.php'; - require_once __DIR__ . '/../plugin/CMSAPI.php'; + // Initialize plugin manager (files already loaded in engine/core/index.php) $this->pluginManager = new PluginManager(__DIR__ . '/../../../plugins'); $api = new CMSAPI($this); $this->pluginManager->setAPI($api); @@ -187,10 +185,10 @@ class CodePressCMS { if ($item[0] === '.') continue; // Skip language-specific content that doesn't match current language - if (preg_match('/^(nl|en)\./', $item)) { - $langPrefix = substr($item, 0, 2); - if (($langPrefix === 'nl' && $this->currentLanguage !== 'nl') || - ($langPrefix === 'en' && $this->currentLanguage !== 'en')) { + $availableLangs = array_keys($this->getAvailableLanguages()); + $langPattern = '/^(' . implode('|', $availableLangs) . ')\./'; + if (preg_match($langPattern, $item, $langMatch)) { + if ($langMatch[1] !== $this->currentLanguage) { continue; } } @@ -261,7 +259,7 @@ class CodePressCMS { $this->searchResults[] = [ 'title' => $title, 'path' => $relativePath, - 'url' => '?page=' . $relativePath, + 'url' => '?page=' . $relativePath . '&lang=' . $this->currentLanguage, 'snippet' => $this->createSnippet($content, $query) ]; } @@ -307,10 +305,6 @@ class CodePressCMS { } $page = $_GET['page'] ?? $this->config['default_page']; - // Sanitize page parameter to prevent XSS - $page = htmlspecialchars($page, ENT_QUOTES, 'UTF-8'); - // Prevent path traversal - $page = str_replace(['../', '..\\', '..'], '', $page); // Limit length $page = substr($page, 0, 255); // Only remove file extension at the end, not all dots @@ -318,6 +312,13 @@ class CodePressCMS { $filePath = $this->config['content_dir'] . '/' . $pageWithoutExt; + // Prevent path traversal using realpath validation + $realContentDir = realpath($this->config['content_dir']); + $realFilePath = realpath($filePath); + if ($realFilePath && $realContentDir && strpos($realFilePath, $realContentDir) !== 0) { + return $this->getError404(); + } + // Check if directory exists FIRST (directories take precedence over files) if (is_dir($filePath)) { return $this->getDirectoryListing($pageWithoutExt, $filePath); @@ -325,13 +326,6 @@ class CodePressCMS { $actualFilePath = null; - // Check if directory exists first (directories take precedence over files) - if (is_dir($filePath)) { - $directoryResult = $this->getDirectoryListing($pageWithoutExt, $filePath); - - return $directoryResult; - } - // Check for exact file matches if no directory found if (file_exists($filePath . '.md')) { $actualFilePath = $filePath . '.md'; @@ -509,10 +503,7 @@ class CodePressCMS { $title = trim($matches[1]); } - // Include autoloader - require_once __DIR__ . '/../../../vendor/autoload.php'; - - // Configure CommonMark environment + // Configure CommonMark environment (autoloader already loaded in bootstrap) $config = [ 'html_input' => 'strip', 'allow_unsafe_links' => false, @@ -584,7 +575,7 @@ class CodePressCMS { return $text; // Don't link existing links, current page title, or H1 headings } - return '' . $text . ''; + return '' . $text . ''; }; $content = preg_replace_callback($pattern, $replacement, $content); @@ -604,11 +595,6 @@ class CodePressCMS { return $pages; } - /** - * Get all page names from content directory (for navigation) - * - * @return array Associative array of page paths to display names - */ /** * Recursively scan for page titles in directory * @@ -647,37 +633,6 @@ class CodePressCMS { } } - /** - * Recursively scan for page names in directory (for navigation) - * - * @param string $dir Directory to scan - * @param string $prefix Relative path prefix - * @param array &$pages Reference to pages array to populate - * @return void - */ - private function scanForPageNames($dir, $prefix, &$pages) { - if (!is_dir($dir)) return; - - $items = scandir($dir); - sort($items); - - foreach ($items as $item) { - if ($item[0] === '.') continue; - - $path = $dir . '/' . $item; - $relativePath = $prefix ? $prefix . '/' . $item : $item; - - if (is_dir($path)) { - $this->scanForPageNames($path, $relativePath, $pages); - } elseif (preg_match('/\.(md|php|html)$/', $item)) { - // Use filename without extension as display name - $displayName = preg_replace('/\.[^.]+$/', '', $item); - $pagePath = preg_replace('/\.[^.]+$/', '', $relativePath); - $pages[$pagePath] = $this->formatDisplayName($displayName); - } - } - } - /** * Format display name from filename * @@ -685,39 +640,33 @@ class CodePressCMS { * @return string Formatted display name */ private function formatDisplayName($filename) { - - - // Remove language prefixes (nl. or en.) from display names - if (preg_match('/^(nl|en)\.(.+)$/', $filename, $matches)) { + // Remove language prefixes dynamically based on available languages + $availableLangs = array_keys($this->getAvailableLanguages()); + $langPattern = '/^(' . implode('|', $availableLangs) . ')\.(.+)$/'; + if (preg_match($langPattern, $filename, $matches)) { $filename = $matches[2]; } - // Remove language prefixes from directory names (nl.php-testen -> php-testen) - if (preg_match('/^(nl|en)\.php-(.+)$/', $filename, $matches)) { - $filename = 'php-' . $matches[2]; - } - // Remove file extensions (.md, .php, .html) from display names $filename = preg_replace('/\.(md|php|html)$/', '', $filename); - // Handle special cases first (only for exact filenames, not directories) - // These should only apply to actual files, not directory names - if (strtolower($filename) === 'phpinfo' && !preg_match('/\//', $filename)) { - return 'phpinfo'; - } - if (strtolower($filename) === 'ict' && !preg_match('/\//', $filename)) { - return 'ICT'; + // Handle special cases (case-sensitive display names) + $specialCases = [ + 'phpinfo' => 'phpinfo', + 'ict' => 'ICT', + ]; + if (isset($specialCases[strtolower($filename)])) { + return $specialCases[strtolower($filename)]; } - // Replace hyphens and underscores with spaces + // Replace hyphens and underscores with spaces, then title case $name = str_replace(['-', '_'], ' ', $filename); - - // Convert to title case (first letter uppercase, rest lowercase) $name = ucwords(strtolower($name)); - // Handle other special cases - $name = str_replace('Phpinfo', 'phpinfo', $name); - $name = str_replace('Ict', 'ICT', $name); + // Post-process special cases in compound names + foreach ($specialCases as $lower => $correct) { + $name = str_ireplace(ucfirst($lower), $correct, $name); + } return $name; } @@ -866,10 +815,7 @@ private function getGuidePage() { $metadata = $parsed['metadata']; $contentWithoutMeta = $parsed['content']; - // Include autoloader - require_once __DIR__ . '/../../../vendor/autoload.php'; - - // Configure CommonMark environment + // Configure CommonMark environment (autoloader already loaded in bootstrap) $config = [ 'html_input' => 'strip', 'allow_unsafe_links' => false, @@ -993,8 +939,6 @@ private function getGuidePage() { $hasContent = true; } - $content .= ''; - if (!$hasContent) { $content .= '
' . $this->t('directory_empty') . '.
'; } @@ -1168,7 +1112,7 @@ private function getGuidePage() { */ public function generateBreadcrumb() { // Sidebar toggle button (shown before home icon in breadcrumb) - $sidebarToggle = ''; + $sidebarToggle = ''; if (isset($_GET['search'])) { return ''; @@ -1194,14 +1138,15 @@ private function getGuidePage() { foreach ($parts as $i => $part) { $currentPath .= ($currentPath ? '/' : '') . $part; - $title = ucfirst($part); + $title = htmlspecialchars(ucfirst($part), ENT_QUOTES, 'UTF-8'); + $safePath = htmlspecialchars($currentPath, ENT_QUOTES, 'UTF-8'); if ($i === count($parts) - 1) { // Last part - active page $breadcrumb .= '