Skip to content

Commit aa6a4de

Browse files
authored
Merge pull request #14 from spcent/claude/wonderful-wozniak-0qp911
security: fix critical/high/medium vulnerabilities (authcode AES-256-GCM, CSRF timing, cookies, UUID, logging)
2 parents ffcf0b2 + 93abce7 commit aa6a4de

8 files changed

Lines changed: 173 additions & 86 deletions

File tree

library/Plume/Engine/ActionInvoker.php

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,32 @@ public static function invoke(string $className, string $requestUri): mixed
4444
*/
4545
public static function sanitizeForLog(array $request): array
4646
{
47-
static $sensitive = ['password', 'passwd', 'pass', 'token', 'secret', 'card_no', 'cvv'];
48-
return array_diff_key($request, array_flip($sensitive));
47+
static $sensitive = [
48+
// Credentials
49+
'password', 'passwd', 'pass', 'pwd', 'new_password', 'old_password', 'confirm_password',
50+
// Tokens & keys
51+
'token', 'secret', 'api_key', 'apikey', 'access_token', 'refresh_token',
52+
'auth_token', 'session_token', 'csrf_token', 'plume_csrf',
53+
// Payment
54+
'card_no', 'card_number', 'cvv', 'cvc', 'expiry', 'bank_account', 'bank_card',
55+
// PII
56+
'id_card', 'id_number', 'ssn', 'social_security', 'passport', 'license_number',
57+
'phone', 'mobile', 'email', 'birthday', 'date_of_birth',
58+
// Private keys
59+
'private_key', 'encryption_key', 'signing_key',
60+
];
61+
$result = array_diff_key($request, array_flip($sensitive));
62+
// Also redact any key containing these substrings (case-insensitive)
63+
$patterns = ['pass', 'secret', 'token', 'key', 'card', 'cvv', 'ssn', 'id_card'];
64+
foreach ($result as $k => $v) {
65+
$lower = strtolower((string) $k);
66+
foreach ($patterns as $pattern) {
67+
if (str_contains($lower, $pattern)) {
68+
$result[$k] = '***REDACTED***';
69+
break;
70+
}
71+
}
72+
}
73+
return $result;
4974
}
5075
}

library/Plume/Engine/Engine.php

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,14 @@ public function init(): void
147147
// session for every request after calling resetForWorker().
148148
if (!IS_CLI && true === C('USE_SESSION') && PHP_SESSION_NONE === session_status()
149149
&& !headers_sent()) {
150+
$isHttps = (!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] !== 'off')
151+
|| (($_SERVER['SERVER_PORT'] ?? '') === '443');
152+
ini_set('session.cookie_httponly', '1');
153+
ini_set('session.use_strict_mode', '1');
154+
ini_set('session.cookie_samesite', 'Lax');
155+
if ($isHttps) {
156+
ini_set('session.cookie_secure', '1');
157+
}
150158
session_start();
151159
}
152160

@@ -535,20 +543,16 @@ public function _error(\Throwable $e): void
535543
}
536544

537545
if ('production' === $this->get('plumephp.env')) {
538-
$msg = sprintf(
539-
'<h1>500 Internal Server Error</h1>'.
540-
'<h3>%s (%s)</h3>',
541-
$e->getMessage(),
542-
$e->getCode()
543-
);
546+
$msg = '<h1>500 Internal Server Error</h1>'
547+
. '<p>An unexpected error occurred. Please try again later.</p>';
544548
} else {
545549
$msg = sprintf(
546550
'<h1>500 Internal Server Error</h1>'.
547551
'<h3>%s (%s)</h3>'.
548552
'<pre>%s</pre>',
549-
$e->getMessage(),
550-
$e->getCode(),
551-
$e->getTraceAsString()
553+
htmlspecialchars($e->getMessage(), ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8'),
554+
htmlspecialchars((string) $e->getCode(), ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8'),
555+
htmlspecialchars($e->getTraceAsString(), ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8')
552556
);
553557
}
554558

@@ -690,10 +694,17 @@ public function _biz(array $params = []): mixed
690694
throw new \Exception('Wrong parameter format, missing path');
691695
}
692696

693-
// Special character processing
694-
$bizPath = str_replace('..', '', (string) $bizPathRaw);
695-
$bizPath = str_replace('/', '', $bizPath);
696-
$bizPath = str_replace('\\', '', $bizPath);
697+
// Validate path using a whitelist: only alphanumerics, underscores, hyphens, and dots
698+
$bizPath = (string) $bizPathRaw;
699+
if (!preg_match('/^[a-zA-Z0-9_\-\.]+$/', $bizPath)) {
700+
throw new \Exception('Wrong parameter format, invalid characters in path');
701+
}
702+
// Prevent path traversal segments
703+
foreach (explode('.', $bizPath) as $segment) {
704+
if ($segment === '' || $segment === '..') {
705+
throw new \Exception('Wrong parameter format, invalid path segment');
706+
}
707+
}
697708
$names = explode('.', $bizPath, 20);
698709
$count = count($names);
699710
if ($count < 2) {

library/Plume/Http/Router.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ private function saveCache(): void
154154
}
155155
$cacheDir = dirname($this->cacheFile);
156156
if (!is_dir($cacheDir)) {
157-
mkdir($cacheDir, 0755, true);
157+
mkdir($cacheDir, 0700, true);
158158
}
159159
$tmp = $this->cacheFile . '.' . getmypid() . '.tmp';
160160
file_put_contents(

library/Plume/Support/Logger.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ public function __construct(
3636
$this->logPath = C('PLUME_LOG_PATH') ?: LOG_PATH;
3737
}
3838
if (!is_dir($this->logPath)) {
39-
mkdir($this->logPath, 0755, true);
39+
mkdir($this->logPath, 0700, true);
4040
}
4141

4242
// Flush buffered DEBUG/INFO/NOTICE logs on shutdown so fatal errors

library/Plume/Support/View.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ public function render(string $file, ?array $data = [], string|false $layout = '
135135
? $this->getCompiledTemplate($this->content)
136136
: $this->content;
137137

138-
extract($this->vars);
138+
extract($this->vars, EXTR_SKIP);
139139
if ('' === $layout || false === $layout) {
140140
include $includeFile;
141141
} else {

library/PlumeHelper.php

Lines changed: 72 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -128,22 +128,30 @@ public static function curlGetContents(
128128

129129
public static function redirect(string $url, int $time = 0, string $msg = ''): never
130130
{
131-
$url = str_replace(["\n", "\r"], '', $url);
131+
$url = str_replace(["\n", "\r", "\0"], '', $url);
132+
133+
// Block javascript: and data: URIs which cannot be safe redirect targets
134+
if (preg_match('/^\s*(?:javascript|data|vbscript):/i', $url)) {
135+
$url = '/';
136+
}
137+
132138
if (empty($msg)) {
133-
$msg = "系统将在{$time}秒之后自动跳转到{$url}";
139+
$escapedUrl = htmlspecialchars($url, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8');
140+
$msg = "系统将在{$time}秒之后自动跳转到{$escapedUrl}";
134141
}
135142
if (!headers_sent()) {
136143
if (0 === $time) {
137144
header('Location: ' . $url);
138145
} else {
139146
header("refresh:{$time};url={$url}");
140-
echo $msg;
147+
echo htmlspecialchars($msg, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8');
141148
}
142149
exit();
143150
} else {
144-
$str = "<meta http-equiv='Refresh' content='{$time};URL={$url}'>";
151+
$safeUrl = htmlspecialchars($url, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8');
152+
$str = "<meta http-equiv='Refresh' content='{$time};URL={$safeUrl}'>";
145153
if ($time != 0) {
146-
$str .= $msg;
154+
$str .= htmlspecialchars($msg, ENT_QUOTES | ENT_SUBSTITUTE, 'UTF-8');
147155
}
148156
exit($str);
149157
}
@@ -245,7 +253,7 @@ public static function signature(array $datas, string $key): string
245253
$tmp[] = $k . '=' . $v;
246254
}
247255
}
248-
return md5(implode('&', $tmp) . '&key=' . $key);
256+
return hash_hmac('sha256', implode('&', $tmp), $key);
249257
}
250258

251259
public static function htmlFilter(string $html): string
@@ -260,56 +268,56 @@ public static function htmlFilter(string $html): string
260268
}
261269

262270
/**
263-
* @deprecated since PlumePHP 1.4.0 — uses MD5-based RC4 stream cipher (Discuz legacy).
264-
* Replace with sodium_crypto_secretbox() or openssl_encrypt('AES-256-GCM', ...).
265-
* This function will be removed in a future major version.
271+
* Encrypt or decrypt a string using AES-256-GCM (authenticated encryption).
272+
*
273+
* Encoding: base64url(nonce[12] + tag[16] + ciphertext) for ENCODE,
274+
* or the same layout for DECODE.
275+
*
276+
* The $expiry parameter is embedded as a 10-digit Unix timestamp prefix
277+
* inside the plaintext (0 = no expiry), preserving backward-compatible
278+
* semantics with the legacy RC4-based implementation this replaced.
279+
*
280+
* @param string $string Plaintext (ENCODE) or ciphertext (DECODE)
281+
* @param string $operation 'ENCODE' or 'DECODE'
282+
* @param string $key Secret key (min 16 chars); falls back to APP_SECRET
283+
* @param int $expiry Seconds until the token expires (0 = forever)
284+
* @return string Ciphertext on ENCODE, plaintext on DECODE, '' on failure
266285
*/
267286
public static function authcode(string $string, string $operation = 'DECODE', string $key = '', int $expiry = 0): string
268287
{
269-
$ckeyLength = 4;
270-
$key = md5($key ?: 'plumephp');
271-
$keya = md5(substr($key, 0, 16));
272-
$keyb = md5(substr($key, 16, 16));
273-
// @phpstan-ignore-next-line ($ckeyLength is intentionally a constant 4 here)
274-
$keyc = $ckeyLength
275-
? ($operation == 'DECODE' ? substr($string, 0, $ckeyLength) : substr(md5(microtime()), -$ckeyLength))
276-
: '';
277-
$cryptkey = $keya . md5($keya . $keyc);
278-
$keyLength = strlen($cryptkey);
279-
$string = $operation == 'DECODE'
280-
? base64_decode(substr($string, $ckeyLength))
281-
: sprintf('%010d', $expiry ? $expiry + time() : 0) . substr(md5($string . $keyb), 0, 16) . $string;
282-
$stringLength = strlen($string);
283-
$result = '';
284-
$box = range(0, 255);
285-
$rndkey = [];
286-
for ($i = 0; $i <= 255; $i++) {
287-
$rndkey[$i] = ord($cryptkey[$i % $keyLength]);
288-
}
289-
for ($j = $i = 0; $i < 256; $i++) {
290-
$j = ($j + $box[$i] + $rndkey[$i]) % 256;
291-
$tmp = $box[$i];
292-
$box[$i] = $box[$j];
293-
$box[$j] = $tmp;
294-
}
295-
for ($a = $j = $i = 0; $i < $stringLength; $i++) {
296-
$a = ($a + 1) % 256;
297-
$j = ($j + $box[$a]) % 256;
298-
$tmp = $box[$a];
299-
$box[$a] = $box[$j];
300-
$box[$j] = $tmp;
301-
$result .= chr((int)(ord($string[$i]) ^ ($box[($box[$a] + $box[$j]) % 256])));
302-
}
303-
if ($operation == 'DECODE') {
304-
$expireTs = (int) substr($result, 0, 10);
305-
if (($expireTs === 0 || $expireTs - time() > 0)
306-
&& substr($result, 10, 16) == substr(md5(substr($result, 26) . $keyb), 0, 16)
307-
) {
308-
return substr($result, 26);
288+
$rawKey = $key ?: (string) getenv('APP_SECRET') ?: 'plumephp-insecure-fallback-key!!';
289+
// Derive a 256-bit key via SHA-256 so any key length is accepted safely
290+
$derivedKey = hash('sha256', $rawKey, true);
291+
292+
if (strtoupper($operation) === 'ENCODE') {
293+
$expireTs = $expiry > 0 ? sprintf('%010d', time() + $expiry) : '0000000000';
294+
$plaintext = $expireTs . $string;
295+
$nonce = random_bytes(12);
296+
$tag = '';
297+
$cipher = openssl_encrypt($plaintext, 'aes-256-gcm', $derivedKey, OPENSSL_RAW_DATA, $nonce, $tag, '', 16);
298+
if ($cipher === false) {
299+
return '';
309300
}
301+
return rtrim(strtr(base64_encode($nonce . $tag . $cipher), '+/', '-_'), '=');
302+
}
303+
304+
// DECODE
305+
$raw = base64_decode(strtr($string, '-_', '+/') . str_repeat('=', (4 - strlen($string) % 4) % 4), true);
306+
if ($raw === false || strlen($raw) < 29) { // 12 nonce + 16 tag + 1 min payload
310307
return '';
311308
}
312-
return $keyc . str_replace('=', '', base64_encode($result));
309+
$nonce = substr($raw, 0, 12);
310+
$tag = substr($raw, 12, 16);
311+
$cipher = substr($raw, 28);
312+
$plain = openssl_decrypt($cipher, 'aes-256-gcm', $derivedKey, OPENSSL_RAW_DATA, $nonce, $tag);
313+
if ($plain === false) {
314+
return '';
315+
}
316+
$expireTs = (int) substr($plain, 0, 10);
317+
if ($expireTs !== 0 && $expireTs < time()) {
318+
return '';
319+
}
320+
return substr($plain, 10);
313321
}
314322

315323
// -----------------------------------------------------------------------
@@ -335,10 +343,20 @@ public static function arrayMergeDeep(array &$arr1, array $arr2): void
335343

336344
public static function uuid(string $prefix = ''): string
337345
{
338-
$time = md5(microtime());
339-
$rand1 = md5(substr($time, rand(0, 10), rand(22, 32)));
340-
$rand2 = md5(substr($rand1, rand(0, 10), rand(22, 32)));
341-
return strtolower(md5($prefix . uniqid($prefix) . $time . $rand1 . $rand2));
346+
$bytes = random_bytes(16);
347+
// Set version 4 (random) and variant bits per RFC 4122
348+
$bytes[6] = chr((ord($bytes[6]) & 0x0f) | 0x40);
349+
$bytes[8] = chr((ord($bytes[8]) & 0x3f) | 0x80);
350+
$hex = bin2hex($bytes);
351+
$uuid = sprintf(
352+
'%s-%s-%s-%s-%s',
353+
substr($hex, 0, 8),
354+
substr($hex, 8, 4),
355+
substr($hex, 12, 4),
356+
substr($hex, 16, 4),
357+
substr($hex, 20, 12)
358+
);
359+
return $prefix ? $prefix . '-' . $uuid : $uuid;
342360
}
343361

344362
public static function strcut(string $str, int $len, string $ext = '', int $zhLen = 0): string

library/core/Plume/Libs/Action.php

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,8 @@ public function run(): mixed
196196

197197
$this->csrfToken = $this->getCookie($this->csrfTokenKey);
198198
if ($this->csrfValidate && C('PLUME_PHP_ENV') !== 'testing' && !$this->validateCsrfToken()) {
199-
header('HTTP/1.1 401 Unauthorized');
200-
$this->error("Unauthorized");
199+
header('HTTP/1.1 403 Forbidden');
200+
$this->error("Forbidden");
201201
}
202202

203203
$this->createCsrfToken();
@@ -485,15 +485,39 @@ public function getCookie(?string $key = null)
485485

486486
/**
487487
* Set a cookie on the response.
488-
* @param string $key Cookie name
489-
* @param string $value Cookie value
490-
* @param int $expire Lifetime in seconds (default 86400)
491-
* @param string $path Cookie path
492-
* @param string $domain Cookie domain
488+
* @param string $key Cookie name
489+
* @param string $value Cookie value
490+
* @param int $expire Lifetime in seconds (default 86400)
491+
* @param string $path Cookie path
492+
* @param string $domain Cookie domain
493+
* @param bool $httpOnly Prevent JavaScript access (default true)
494+
* @param bool $secure Transmit over HTTPS only (default auto-detected)
495+
* @param 'Lax'|'lax'|'Strict'|'strict'|'None'|'none' $sameSite SameSite policy (default 'Lax')
493496
*/
494-
public function setCookie(string $key, string $value, int $expire = 86400, string $path = '/', string $domain = ''): void
495-
{
496-
setcookie($key, $value, time() + $expire, $path, $domain);
497+
public function setCookie(
498+
string $key,
499+
string $value,
500+
int $expire = 86400,
501+
string $path = '/',
502+
string $domain = '',
503+
bool $httpOnly = true,
504+
bool $secure = false,
505+
string $sameSite = 'Lax'
506+
): void {
507+
$isHttps = (!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] !== 'off')
508+
|| (($_SERVER['SERVER_PORT'] ?? '') === '443');
509+
/** @var array{expires: int, path: string, domain: string, secure: bool, httponly: bool, samesite: 'Lax'|'lax'|'Strict'|'strict'|'None'|'none'} $options */
510+
$options = [
511+
'expires' => time() + $expire,
512+
'path' => $path,
513+
'domain' => $domain,
514+
'secure' => $secure || $isHttps,
515+
'httponly' => $httpOnly,
516+
'samesite' => in_array($sameSite, ['Lax', 'lax', 'Strict', 'strict', 'None', 'none'], true)
517+
? $sameSite
518+
: 'Lax',
519+
];
520+
setcookie($key, $value, $options);
497521
}
498522

499523
/**
@@ -629,7 +653,7 @@ public function validateCsrfToken()
629653
$mask = str_pad($mask, $n2, $n1 === 0 ? ' ' : $mask);
630654
}
631655
$token = $mask ^ $token;
632-
return $token === $trueToken;
656+
return hash_equals($trueToken, $token);
633657
}
634658

635659
protected function beforeRun(): bool

tests/PlumeHelperTest.php

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,16 +94,25 @@ public function testHtmlFilter(): void
9494
// Data / String
9595
// -----------------------------------------------------------------------
9696

97-
public function testUuidIsLowerHex(): void
97+
public function testUuidIsRfc4122(): void
9898
{
9999
$id = PlumeHelper::uuid();
100-
$this->assertMatchesRegularExpression('/^[0-9a-f]+$/', $id);
100+
// RFC 4122 v4 format: xxxxxxxx-xxxx-4xxx-[89ab]xxx-xxxxxxxxxxxx
101+
$this->assertMatchesRegularExpression(
102+
'/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/',
103+
$id
104+
);
101105
}
102106

103107
public function testUuidWithPrefix(): void
104108
{
105109
$id = PlumeHelper::uuid('usr');
106-
$this->assertMatchesRegularExpression('/^[0-9a-f]+$/', $id);
110+
$this->assertStringStartsWith('usr-', $id);
111+
$uuid = substr($id, 4);
112+
$this->assertMatchesRegularExpression(
113+
'/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/',
114+
$uuid
115+
);
107116
}
108117

109118
public function testStrcut(): void

0 commit comments

Comments
 (0)