feat: sanitize root folder also in php error messages (#3262)

This commit is contained in:
Dag 2023-03-06 21:47:25 +01:00 committed by GitHub
parent a01c1f6ab0
commit 007f2b2d8a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 38 additions and 17 deletions

View File

@ -34,8 +34,8 @@ final class Logger
unset($context['e']); unset($context['e']);
$context['type'] = get_class($e); $context['type'] = get_class($e);
$context['code'] = $e->getCode(); $context['code'] = $e->getCode();
$context['message'] = $e->getMessage(); $context['message'] = sanitize_root($e->getMessage());
$context['file'] = trim_path_prefix($e->getFile()); $context['file'] = sanitize_root($e->getFile());
$context['line'] = $e->getLine(); $context['line'] = $e->getLine();
$context['url'] = get_current_url(); $context['url'] = get_current_url();
$context['trace'] = trace_to_call_points(trace_from_exception($e)); $context['trace'] = trace_to_call_points(trace_from_exception($e));
@ -58,6 +58,7 @@ final class Logger
} }
} }
} }
// Intentionally not sanitizing $message
$text = sprintf( $text = sprintf(
"[%s] rssbridge.%s %s %s\n", "[%s] rssbridge.%s %s %s\n",
now()->format('Y-m-d H:i:s'), now()->format('Y-m-d H:i:s'),
@ -65,6 +66,7 @@ final class Logger
$message, $message,
$context ? Json::encode($context) : '' $context ? Json::encode($context) : ''
); );
// Log to stderr/stdout whatever that is
error_log($text); error_log($text);
} }
} }

View File

@ -34,8 +34,12 @@ final class RssBridge
if ((error_reporting() & $code) === 0) { if ((error_reporting() & $code) === 0) {
return false; return false;
} }
$text = sprintf('%s at %s line %s', $message, trim_path_prefix($file), $line); $text = sprintf(
// Drop the current frame '%s at %s line %s',
sanitize_root($message),
sanitize_root($file),
$line
);
Logger::warning($text); Logger::warning($text);
if (Debug::isEnabled()) { if (Debug::isEnabled()) {
print sprintf("<pre>%s</pre>\n", e($text)); print sprintf("<pre>%s</pre>\n", e($text));
@ -49,8 +53,8 @@ final class RssBridge
$message = sprintf( $message = sprintf(
'Fatal Error %s: %s in %s line %s', 'Fatal Error %s: %s in %s line %s',
$error['type'], $error['type'],
$error['message'], sanitize_root($error['message']),
trim_path_prefix($error['file']), sanitize_root($error['file']),
$error['line'] $error['line']
); );
Logger::error($message); Logger::error($message);

View File

@ -50,8 +50,8 @@ function create_sane_exception_message(\Throwable $e): string
return sprintf( return sprintf(
'%s: %s in %s line %s', '%s: %s in %s line %s',
get_class($e), get_class($e),
$e->getMessage(), sanitize_root($e->getMessage()),
trim_path_prefix($e->getFile()), sanitize_root($e->getFile()),
$e->getLine() $e->getLine()
); );
} }
@ -74,7 +74,7 @@ function trace_from_exception(\Throwable $e): array
$trace = []; $trace = [];
foreach ($frames as $frame) { foreach ($frames as $frame) {
$trace[] = [ $trace[] = [
'file' => trim_path_prefix($frame['file'] ?? ''), 'file' => sanitize_root($frame['file'] ?? ''),
'line' => $frame['line'] ?? null, 'line' => $frame['line'] ?? null,
'class' => $frame['class'] ?? null, 'class' => $frame['class'] ?? null,
'type' => $frame['type'] ?? null, 'type' => $frame['type'] ?? null,
@ -121,9 +121,17 @@ function frame_to_call_point(array $frame): string
* *
* Example: "/home/davidsf/rss-bridge/index.php" => "index.php" * Example: "/home/davidsf/rss-bridge/index.php" => "index.php"
*/ */
function trim_path_prefix(string $filePath): string function sanitize_root(string $filePath): string
{ {
return mb_substr($filePath, mb_strlen(dirname(__DIR__)) + 1); // Root folder of the project e.g. /home/satoshi/repos/rss-bridge
$root = dirname(__DIR__);
return _sanitize_path_name($filePath, $root);
}
function _sanitize_path_name(string $s, string $pathName): string
{
// Remove all occurrences of $pathName in the string
return str_replace(["$pathName/", $pathName], '', $s);
} }
/** /**

View File

@ -16,11 +16,11 @@
</div> </div>
<div> <div>
<strong>Message:</strong> <?= e($e->getMessage()) ?> <strong>Message:</strong> <?= e(sanitize_root($e->getMessage())) ?>
</div> </div>
<div> <div>
<strong>File:</strong> <?= e(trim_path_prefix($e->getFile())) ?> <strong>File:</strong> <?= e(sanitize_root($e->getFile())) ?>
</div> </div>
<div> <div>

View File

@ -41,10 +41,17 @@ final class UtilsTest extends TestCase
$sut->purgeCache(-1); $sut->purgeCache(-1);
} }
public function testTrimFilePath() public function testSanitizePathName()
{ {
$this->assertSame('', trim_path_prefix(dirname(__DIR__))); $this->assertSame('index.php', _sanitize_path_name('/home/satoshi/rss-bridge/index.php', '/home/satoshi/rss-bridge'));
$this->assertSame('tests', trim_path_prefix(__DIR__)); $this->assertSame('tests/UtilsTest.php', _sanitize_path_name('/home/satoshi/rss-bridge/tests/UtilsTest.php', '/home/satoshi/rss-bridge'));
$this->assertSame('tests/UtilsTest.php', trim_path_prefix(__DIR__ . '/UtilsTest.php')); $this->assertSame('bug in lib/kek.php', _sanitize_path_name('bug in /home/satoshi/rss-bridge/lib/kek.php', '/home/satoshi/rss-bridge'));
}
public function testSanitizePathNameInErrorMessage()
{
$raw = 'Error: Argument 1 passed to foo() must be an instance of kk, string given, called in /home/satoshi/rss-bridge/bridges/RumbleBridge.php';
$sanitized = 'Error: Argument 1 passed to foo() must be an instance of kk, string given, called in bridges/RumbleBridge.php';
$this->assertSame($sanitized, _sanitize_path_name($raw, '/home/satoshi/rss-bridge'));
} }
} }