From d1040301f613a600ecf312fe50835d79bc058ceb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Metin=20=C5=9Eim=C5=9Fek?=
<75851971+devsimsek@users.noreply.github.com>
Date: Sun, 5 Jul 2026 17:18:56 +0200
Subject: [PATCH] fix(security): resolve 4 critical security findings.
- SDF-56: Escape output in 404/405 error handlers, set Content-Type
- SDF-58: Wire Cache::normalizeKey() into all facade methods + empty-key guard
- SDF-60: Use EXTR_SKIP in Fuse/Loader extract() to prevent LFI via variable overwrite
- SDF-61: Invert X-Forwarded-For trust default (deny when no trusted proxy)
- Bump version 2.3.0 -> 2.3.1
---
app/handlers/errors.php | 15 +++++++++++++--
index.php | 2 +-
sdf/__init.php | 2 +-
sdf/core/Cache/Cache.php | 31 +++++++++++++++++++++++--------
sdf/core/Fuse.php | 2 +-
sdf/core/Loader.php | 8 ++++----
sdf/core/Request.php | 17 +++++++++--------
wiki/app/handlers.md | 33 +++++++++++++++++++++++++++++++++
wiki/libraries/caching.md | 15 +++++++++++++++
wiki/libraries/fuse.md | 8 ++++++++
wiki/libraries/middleware.md | 16 +++++++++++++++-
wiki/libraries/request.md | 2 +-
wiki/sdf/core.md | 2 ++
13 files changed, 126 insertions(+), 27 deletions(-)
diff --git a/app/handlers/errors.php b/app/handlers/errors.php
index cd8ff1b..034f56d 100644
--- a/app/handlers/errors.php
+++ b/app/handlers/errors.php
@@ -8,8 +8,13 @@
*/
function eh_pathNotFound($requestPath): void
{
+ if (!headers_sent()) {
+ header('HTTP/1.0 404 Not Found');
+ header('Content-Type: text/html; charset=UTF-8');
+ }
if (!is_array($requestPath)) {
- print_r('404 Error, Path ' . $requestPath . ' Not Found.');
+ $safePath = htmlspecialchars($requestPath, ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML5, 'UTF-8');
+ print_r('404 Error, Path ' . $safePath . ' Not Found.');
} else {
print_r('404 Error, Path Not Found.');
}
@@ -25,7 +30,13 @@ function eh_pathNotFound($requestPath): void
*/
function eh_methodNotAllowed($requestPath, $requestMethod): void
{
- print_r("Method " . $requestMethod . " not allowed on this path.");
+ if (!headers_sent()) {
+ header('HTTP/1.0 405 Method Not Allowed');
+ header('Content-Type: text/html; charset=UTF-8');
+ }
+ $safeMethod = htmlspecialchars($requestMethod, ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML5, 'UTF-8');
+ $safePath = is_array($requestPath) ? '' : ' ' . htmlspecialchars($requestPath, ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML5, 'UTF-8');
+ print_r("Method " . $safeMethod . " not allowed on this path" . $safePath . ".");
exit();
}
diff --git a/index.php b/index.php
index d86d800..58e409c 100644
--- a/index.php
+++ b/index.php
@@ -5,7 +5,7 @@
* Copyright devsimsek
* @package SDF
* @file index.php
- * @version v2.3.0
+ * @version v2.3.1
* @author devsimsek
* @copyright Copyright (c) 2022 - 2026, smskSoft, devsimsek
* @license https://opensource.org/licenses/MIT MIT License
diff --git a/sdf/__init.php b/sdf/__init.php
index ad990ba..d160ccb 100644
--- a/sdf/__init.php
+++ b/sdf/__init.php
@@ -22,7 +22,7 @@
print_r('PANIC: sdf is not called by it\'s own script.');
exit(1);
}
-const SDF_VERSION = "2.3.0";
+const SDF_VERSION = "2.3.1";
// Check minimum version requirement of this framework.
// PHP 8.2 or higher is required, framework is tested and compatible up to PHP 8.5
diff --git a/sdf/core/Cache/Cache.php b/sdf/core/Cache/Cache.php
index 8898809..191ad42 100644
--- a/sdf/core/Cache/Cache.php
+++ b/sdf/core/Cache/Cache.php
@@ -45,7 +45,7 @@ class Cache
*/
public static function get(string $key, mixed $default = null): mixed
{
- return self::driver()->get($key, $default);
+ return self::driver()->get(self::normalizeKey($key), $default);
}
/**
@@ -58,7 +58,7 @@ public static function get(string $key, mixed $default = null): mixed
*/
public static function set(string $key, mixed $value, null|int|DateInterval $ttl = null): bool
{
- return self::driver()->set($key, $value, $ttl);
+ return self::driver()->set(self::normalizeKey($key), $value, $ttl);
}
/**
@@ -69,7 +69,7 @@ public static function set(string $key, mixed $value, null|int|DateInterval $ttl
*/
public static function delete(string $key): bool
{
- return self::driver()->delete($key);
+ return self::driver()->delete(self::normalizeKey($key));
}
/**
@@ -91,7 +91,11 @@ public static function clear(): bool
*/
public static function getMultiple(iterable $keys, mixed $default = null): iterable
{
- return self::driver()->getMultiple($keys, $default);
+ $normalized = [];
+ foreach ($keys as $k) {
+ $normalized[] = self::normalizeKey($k);
+ }
+ return self::driver()->getMultiple($normalized, $default);
}
/**
@@ -103,7 +107,11 @@ public static function getMultiple(iterable $keys, mixed $default = null): itera
*/
public static function setMultiple(iterable $values, null|int|DateInterval $ttl = null): bool
{
- return self::driver()->setMultiple($values, $ttl);
+ $normalized = [];
+ foreach ($values as $k => $v) {
+ $normalized[self::normalizeKey($k)] = $v;
+ }
+ return self::driver()->setMultiple($normalized, $ttl);
}
/**
@@ -114,7 +122,11 @@ public static function setMultiple(iterable $values, null|int|DateInterval $ttl
*/
public static function deleteMultiple(iterable $keys): bool
{
- return self::driver()->deleteMultiple($keys);
+ $normalized = [];
+ foreach ($keys as $k) {
+ $normalized[] = self::normalizeKey($k);
+ }
+ return self::driver()->deleteMultiple($normalized);
}
/**
@@ -125,7 +137,7 @@ public static function deleteMultiple(iterable $keys): bool
*/
public static function has(string $key): bool
{
- return self::driver()->has($key);
+ return self::driver()->has(self::normalizeKey($key));
}
/**
@@ -147,7 +159,7 @@ public static function tags(array $tags): CacheDriver
*/
public static function forget(string $key): bool
{
- return self::driver()->delete($key);
+ return self::driver()->delete(self::normalizeKey($key));
}
/**
@@ -248,6 +260,9 @@ public static function ttlToSeconds(null|int|DateInterval $ttl): int
*/
public static function normalizeKey(string $key): string
{
+ if ($key === '') {
+ throw new \InvalidArgumentException('Cache key must not be empty (PSR-16)');
+ }
$key = preg_replace('/[{}()\/\\\@:"]/', '_', $key);
return trim(preg_replace('/[^a-zA-Z0-9_.]/', '', $key));
}
diff --git a/sdf/core/Fuse.php b/sdf/core/Fuse.php
index e5992f2..6c7333b 100644
--- a/sdf/core/Fuse.php
+++ b/sdf/core/Fuse.php
@@ -96,7 +96,7 @@ public function render(
$cacheFile = $cacheDir . md5($path . $viewFile) . '.php';
file_put_contents($cacheFile, $content);
- extract($this->data);
+ extract($this->data, EXTR_SKIP);
ob_start();
require $cacheFile;
return ob_get_clean();
diff --git a/sdf/core/Loader.php b/sdf/core/Loader.php
index 609b464..700a788 100644
--- a/sdf/core/Loader.php
+++ b/sdf/core/Loader.php
@@ -28,18 +28,18 @@ class Loader
/**
* Load a view file.
- * @param string $name
+ * @param string $view
* @param array|object $params
* @param string $directory
* @return bool
* @throws Exception
*/
public function view(
- string $name,
+ string $view,
array|object $params = [],
string $directory = SDF_APP_VIEW
): bool {
- $viewName = $this->normalizeFilename($name);
+ $viewName = $this->normalizeFilename($view);
$useFuse = defined('USE_FUSE') ? (bool) constant('USE_FUSE') : false;
if (!$this->isLoaded($viewName) && file_exists($directory . $viewName)) {
@@ -47,7 +47,7 @@ public function view(
$params = get_object_vars($params);
}
if (is_array($params)) {
- extract($params);
+ extract($params, EXTR_SKIP);
}
$this->load($viewName);
diff --git a/sdf/core/Request.php b/sdf/core/Request.php
index 2dcc182..0ff4234 100644
--- a/sdf/core/Request.php
+++ b/sdf/core/Request.php
@@ -439,15 +439,16 @@ public function port(): int
*/
public function ip(): ?string
{
- $trusted = $_SERVER["SDF_TRUSTED_PROXIES"] ?? "";
$remote = $_SERVER["REMOTE_ADDR"] ?? null;
+ $trusted = $_SERVER["SDF_TRUSTED_PROXIES"] ?? "";
- if ($remote !== null && $trusted !== "") {
- $proxies = explode(",", $trusted);
- $proxies = array_map('trim', $proxies);
- if (!in_array($remote, $proxies, true)) {
- return $remote;
- }
+ if ($trusted === "" || $remote === null) {
+ return $remote;
+ }
+
+ $proxies = array_map('trim', explode(",", $trusted));
+ if (!in_array($remote, $proxies, true)) {
+ return $remote;
}
$forwarded = $_SERVER["HTTP_X_FORWARDED_FOR"] ?? "";
@@ -460,7 +461,7 @@ public function ip(): ?string
}
}
}
- return $remote ?? ($_SERVER["HTTP_CLIENT_IP"] ?? null);
+ return $remote;
}
/**
diff --git a/wiki/app/handlers.md b/wiki/app/handlers.md
index 3c6fc16..c745291 100644
--- a/wiki/app/handlers.md
+++ b/wiki/app/handlers.md
@@ -85,3 +85,36 @@ function to load the configuration file and output it as JSON.
In this section, you learned how to create handlers in SDF. You learned about the basic handler, functions, and using
handlers in controllers.
+
+## Error Handlers (v2.3.1+)
+
+The framework calls two built-in error handlers wired in `sdf/__init.php`:
+
+- `eh_pathNotFound($requestPath)` - called by the Router on 404
+- `eh_methodNotAllowed($requestPath, $requestMethod)` - called on 405
+
+Both live in `app/handlers/errors.php` alongside `eh_errorHandler`. As of **v2.3.1**, these handlers:
+
+- Send the correct HTTP status (`404 Not Found` / `405 Method Not Allowed`) and an explicit `Content-Type: text/html; charset=UTF-8` header before any output.
+- HTML-escape every user-controlled value (`$requestPath`, `$requestMethod`) with `htmlspecialchars($value, ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML5, 'UTF-8')` to prevent reflected XSS.
+
+> Security: Never echo `$requestPath` or `$requestMethod` raw - both come from `$_SERVER['REQUEST_URI']` / `$_SERVER['REQUEST_METHOD']` and are fully attacker-controlled via the raw HTTP request line. If you replace these handlers, follow the same escaping + header pattern shown in `app/handlers/errors.php`.
+
+A hardened 404 handler looks like:
+
+```php
+function eh_pathNotFound($requestPath): void
+{
+ if (!headers_sent()) {
+ header('HTTP/1.0 404 Not Found');
+ header('Content-Type: text/html; charset=UTF-8');
+ }
+ if (!is_array($requestPath)) {
+ $safePath = htmlspecialchars($requestPath, ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML5, 'UTF-8');
+ print_r('404 Error, Path ' . $safePath . ' Not Found.');
+ } else {
+ print_r('404 Error, Path Not Found.');
+ }
+ exit();
+}
+```
diff --git a/wiki/libraries/caching.md b/wiki/libraries/caching.md
index a9c3253..371e8b2 100644
--- a/wiki/libraries/caching.md
+++ b/wiki/libraries/caching.md
@@ -97,6 +97,21 @@ $driver->get('anything', 'fallback'); // 'fallback'
$driver->set('x', 'y'); // false
```
+### Key normalization (v2.3.1+)
+
+Every facade method (`get`, `set`, `delete`, `has`, `forget`, `getMultiple`, `setMultiple`, `deleteMultiple`) runs the key through `Cache::normalizeKey()` before delegating to the driver. This enforces the PSR-16 charset and closes injection vectors into Redis hash tags, Memcached keyspaces, and the file-driver path.
+
+- Reserved PSR-16 characters `{}()/\@:"` are replaced with `_`.
+- Any character outside `[a-zA-Z0-9_.]` is stripped.
+- Empty keys throw `InvalidArgumentException` (PSR-16 §1.3 forbids empty keys).
+
+```php
+Cache::normalizeKey('user:{42}/posts'); // 'user_42_posts'
+Cache::normalizeKey(''); // throws \InvalidArgumentException
+```
+
+> If you cache objects, note that `FileDriver` currently unserializes with `allowed_classes => true`. Keep your cache directory private (`app/cache/` rather than `/tmp`) and never cache content that could be tampered with by another tenant. A future release will switch the default to `allowed_classes => false` for defense in depth.
+
## Drivers
| Driver | Extension required | Storage |
diff --git a/wiki/libraries/fuse.md b/wiki/libraries/fuse.md
index 8e64b73..9b35ed3 100644
--- a/wiki/libraries/fuse.md
+++ b/wiki/libraries/fuse.md
@@ -31,6 +31,12 @@ In the view, data is available as extracted PHP variables:
{{ $total }} posts found.
```
+### Safe variable extraction (v2.3.1+)
+
+Both `Fuse::render()` and `Loader::view()` extract view data with `EXTR_SKIP` instead of the default `EXTR_OVERWRITE`. This means **view data cannot overwrite the framework's internal local variables** (`$cacheFile`, `$cacheDir`, `$path`, `$viewFile`, `$viewName`, `$directory`, `$useFuse`). Without this guard, passing user-controlled keys like `['cacheFile' => '/etc/passwd']` into a view would let an attacker hijack the `require` path (LFI/RCE).
+
+> Safe pattern: `Fuse::with($_REQUEST)->render(...)` is now safe; previously it was not. You should still prefer `with(['specific' => $value])` over passing raw superglobals.
+
## Variable Interpolation
`{{ $var }}` - escaped via `htmlspecialchars`. Safe by default.
@@ -40,6 +46,8 @@ In the view, data is available as extracted PHP variables:
Email: {{ $user['email'] }}
```
+> As of v2.3.1, `{{ $var }}` compiles to `htmlspecialchars($var, ENT_QUOTES)`. Note this omits `ENT_SUBSTITUTE` and an explicit `'UTF-8'` charset argument - tracked for a future hardening release. For now, ensure your app serves UTF-8 to avoid edge cases with invalid byte sequences.
+
## Directives
### `@If / @ElseIf / @Else / @endIf`
diff --git a/wiki/libraries/middleware.md b/wiki/libraries/middleware.md
index bbe98ee..950a900 100644
--- a/wiki/libraries/middleware.md
+++ b/wiki/libraries/middleware.md
@@ -137,13 +137,27 @@ When `allow_credentials` is `true`, wildcard origins (`*`) are rejected — you
## Rate Limit Middleware
-The built-in `RateLimitMiddleware` uses the Cache facade (60 req/min per IP+route by default). IP detection respects trusted proxies — configure via the `SDF_TRUSTED_PROXIES` server variable:
+The built-in `RateLimitMiddleware` uses the Cache facade (60 req/min per IP+route by default). IP detection respects trusted proxies - configure via the `SDF_TRUSTED_PROXIES` server variable:
```php
// In index.php or .env
$_SERVER['SDF_TRUSTED_PROXIES'] = '10.0.0.1,10.0.0.2';
```
+### Default-deny X-Forwarded-For (v2.3.1+)
+
+As of **v2.3.1**, `Request::ip()` follows a **default-deny** policy. When `SDF_TRUSTED_PROXIES` is unset (the default on a fresh install and the bundled `compose.yaml`/`Caddyfile`), only `REMOTE_ADDR` is returned - **`X-Forwarded-For` and `HTTP_CLIENT_IP` are never consulted**. This closes a rate-limit / IP-ban bypass where any client could spoof its IP by setting `X-Forwarded-For: ` per request.
+
+Behaviour summary:
+
+| `SDF_TRUSTED_PROXIES` | `REMOTE_ADDR` matches? | IP returned |
+|---|---|---|
+| unset / empty | n/a | `REMOTE_ADDR` (XFF ignored) |
+| set | yes | first valid IP from `X-Forwarded-For` |
+| set | no | `REMOTE_ADDR` (XFF ignored - untrusted proxy) |
+
+The legacy `HTTP_CLIENT_IP` fallback has been removed entirely. If you deployed SDF behind a TLS-terminating proxy before v2.3.1 and relied on implicit XFF trust, you **must** set `SDF_TRUSTED_PROXIES` to the proxy's IP after upgrading, or rate limiting / IP-based auth will key on the proxy's address for every client.
+
Untrusted `X-Forwarded-For` headers are ignored.
## Rate Limit Middleware Example
diff --git a/wiki/libraries/request.md b/wiki/libraries/request.md
index 16f8375..f7d8589 100644
--- a/wiki/libraries/request.md
+++ b/wiki/libraries/request.md
@@ -332,7 +332,7 @@ Get the request origin (scheme + host).
#### `ip(): ?string`
-Get the client IP address. Checks `X-Forwarded-For`, `HTTP_CLIENT_IP`, and `REMOTE_ADDR`.
+Get the client IP address. Returns `REMOTE_ADDR` directly when no trusted proxy is configured. When `SDF_TRUSTED_PROXIES` is set and `REMOTE_ADDR` matches an entry in that list, the first valid IP from `X-Forwarded-For` is returned instead. As of v2.3.1, `X-Forwarded-For` and `HTTP_CLIENT_IP` are never consulted unless `SDF_TRUSTED_PROXIES` is explicitly configured (default-deny).
- **Returns:** `string|null`
diff --git a/wiki/sdf/core.md b/wiki/sdf/core.md
index 894d874..93937d8 100644
--- a/wiki/sdf/core.md
+++ b/wiki/sdf/core.md
@@ -79,6 +79,8 @@ $this->load->library('CsvExporter'); // require app/libraries/CsvExporte
$this->load->config('mail'); // load app/config/mail.php
```
+> As of **v2.3.1**, `Loader::view()` first parameter is named `$view` (was `$name`). The rename is positional-only - existing callers `$this->load->view('home')` are unaffected. View data is now extracted with `EXTR_SKIP` (see [Fuse - Safe variable extraction](../libraries/fuse.md#safe-variable-extraction-v231)).
+
## Controller Base
All controllers extend `SDF\Controller`. Properties auto-injected: