From 007f2b2d8ad7eac4312ba3cee8f5ef61a745375e Mon Sep 17 00:00:00 2001
From: Dag <me@dvikan.no>
Date: Mon, 6 Mar 2023 21:47:25 +0100
Subject: [PATCH] feat: sanitize root folder also in php error messages (#3262)

---
 lib/Logger.php           |  6 ++++--
 lib/RssBridge.php        | 12 ++++++++----
 lib/utils.php            | 18 +++++++++++++-----
 templates/error.html.php |  4 ++--
 tests/UtilsTest.php      | 15 +++++++++++----
 5 files changed, 38 insertions(+), 17 deletions(-)

diff --git a/lib/Logger.php b/lib/Logger.php
index bcdec7cc..9bbdd512 100644
--- a/lib/Logger.php
+++ b/lib/Logger.php
@@ -34,8 +34,8 @@ final class Logger
             unset($context['e']);
             $context['type'] = get_class($e);
             $context['code'] = $e->getCode();
-            $context['message'] = $e->getMessage();
-            $context['file'] = trim_path_prefix($e->getFile());
+            $context['message'] = sanitize_root($e->getMessage());
+            $context['file'] = sanitize_root($e->getFile());
             $context['line'] = $e->getLine();
             $context['url'] = get_current_url();
             $context['trace'] = trace_to_call_points(trace_from_exception($e));
@@ -58,6 +58,7 @@ final class Logger
                 }
             }
         }
+        // Intentionally not sanitizing $message
         $text = sprintf(
             "[%s] rssbridge.%s %s %s\n",
             now()->format('Y-m-d H:i:s'),
@@ -65,6 +66,7 @@ final class Logger
             $message,
             $context ? Json::encode($context) : ''
         );
+        // Log to stderr/stdout whatever that is
         error_log($text);
     }
 }
diff --git a/lib/RssBridge.php b/lib/RssBridge.php
index ce895bf2..62e8acc5 100644
--- a/lib/RssBridge.php
+++ b/lib/RssBridge.php
@@ -34,8 +34,12 @@ final class RssBridge
             if ((error_reporting() & $code) === 0) {
                 return false;
             }
-            $text = sprintf('%s at %s line %s', $message, trim_path_prefix($file), $line);
-            // Drop the current frame
+            $text = sprintf(
+                '%s at %s line %s',
+                sanitize_root($message),
+                sanitize_root($file),
+                $line
+            );
             Logger::warning($text);
             if (Debug::isEnabled()) {
                 print sprintf("<pre>%s</pre>\n", e($text));
@@ -49,8 +53,8 @@ final class RssBridge
                 $message = sprintf(
                     'Fatal Error %s: %s in %s line %s',
                     $error['type'],
-                    $error['message'],
-                    trim_path_prefix($error['file']),
+                    sanitize_root($error['message']),
+                    sanitize_root($error['file']),
                     $error['line']
                 );
                 Logger::error($message);
diff --git a/lib/utils.php b/lib/utils.php
index 41858a09..9eb6f5ec 100644
--- a/lib/utils.php
+++ b/lib/utils.php
@@ -50,8 +50,8 @@ function create_sane_exception_message(\Throwable $e): string
     return sprintf(
         '%s: %s in %s line %s',
         get_class($e),
-        $e->getMessage(),
-        trim_path_prefix($e->getFile()),
+        sanitize_root($e->getMessage()),
+        sanitize_root($e->getFile()),
         $e->getLine()
     );
 }
@@ -74,7 +74,7 @@ function trace_from_exception(\Throwable $e): array
     $trace = [];
     foreach ($frames as $frame) {
         $trace[] = [
-            'file'      => trim_path_prefix($frame['file'] ?? ''),
+            'file'      => sanitize_root($frame['file'] ?? ''),
             'line'      => $frame['line'] ?? null,
             'class'     => $frame['class'] ?? 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"
  */
-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);
 }
 
 /**
diff --git a/templates/error.html.php b/templates/error.html.php
index 59105e64..fb94fc25 100644
--- a/templates/error.html.php
+++ b/templates/error.html.php
@@ -16,11 +16,11 @@
         </div>
 
         <div>
-            <strong>Message:</strong> <?= e($e->getMessage()) ?>
+            <strong>Message:</strong> <?= e(sanitize_root($e->getMessage())) ?>
         </div>
 
         <div>
-            <strong>File:</strong> <?= e(trim_path_prefix($e->getFile())) ?>
+            <strong>File:</strong> <?= e(sanitize_root($e->getFile())) ?>
         </div>
 
         <div>
diff --git a/tests/UtilsTest.php b/tests/UtilsTest.php
index 6c6c5e82..c7705b48 100644
--- a/tests/UtilsTest.php
+++ b/tests/UtilsTest.php
@@ -41,10 +41,17 @@ final class UtilsTest extends TestCase
         $sut->purgeCache(-1);
     }
 
-    public function testTrimFilePath()
+    public function testSanitizePathName()
     {
-        $this->assertSame('', trim_path_prefix(dirname(__DIR__)));
-        $this->assertSame('tests', trim_path_prefix(__DIR__));
-        $this->assertSame('tests/UtilsTest.php', trim_path_prefix(__DIR__ . '/UtilsTest.php'));
+        $this->assertSame('index.php', _sanitize_path_name('/home/satoshi/rss-bridge/index.php', '/home/satoshi/rss-bridge'));
+        $this->assertSame('tests/UtilsTest.php', _sanitize_path_name('/home/satoshi/rss-bridge/tests/UtilsTest.php', '/home/satoshi/rss-bridge'));
+        $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'));
     }
 }