mask the whole url when userinfo and a query credential overlap

Masking userinfo first leaked the tail of a query password holding "@" on URLs with a port, like rtsp://10.0.0.2:554/live?password=pre@suf. When the userinfo span contains a credential query key, the mask now covers the rest of the URL in both clean_camera_user_pass and maskCredentials.

frigate/const.py

@@ -118,12 +118,16 @@ SUPPORTED_RK_SOCS = ["rk3562", "rk3566", "rk3568", "rk3576", "rk3588"]
 REGEX_CAMERA_NAME = r"^[a-zA-Z0-9_-]+$"
 REGEX_RTSP_CAMERA_USER_PASS = r":\/\/[a-zA-Z0-9_-]+:[\S]+@"
 # A "user:" userinfo may hold "/", "#" or "@" in the password, so it runs to the
-# last "@" before the next URL. Anything else stops at the authority end.
-REGEX_URL_USERINFO = r"://(?:[^\s/?#:\[\]]*:(?:(?!://)\S)*|[^\s/?#]+)@"
+# last "@" before the next URL. Anything else stops at the authority end. Group 1
+# is the userinfo and group 2 the rest of the URL.
+REGEX_URL_USERINFO = (
+    r"://((?:[^\s/?#:\[\]]*:(?:(?!://)\S)*|[^\s/?#]+))@((?:(?!://)\S)*)"
+)
 _CREDENTIAL_QUERY_KEYS = (
     r"(?:user(?:name)?|usr|login|pass(?:word|wd)?|pwd"
     r"|(?:[a-z_]*_)?token|(?:[a-z_]*_)?secret|client_private|api_?key)"
 )
+REGEX_CREDENTIAL_QUERY_KEY = rf"(?i)[?&]{_CREDENTIAL_QUERY_KEYS}="
 REGEX_CREDENTIAL_QUERY_PARAM = rf"(?i)([?&]{_CREDENTIAL_QUERY_KEYS}=)[^&\s]*"
 REGEX_MASKED_CREDENTIAL = rf"(?i)://\*:\*@|[?&]{_CREDENTIAL_QUERY_KEYS}=\*(?![^&\s])"
 REGEX_JSON = re.compile(r"^\s*\{")

frigate/test/test_camera_pw.py

@@ -64,8 +64,14 @@ class TestUserPassMasking(unittest.TestCase):
             "rtsp://admin:12#abc@10.0.0.2/x": "rtsp://*:*@10.0.0.2/x",
             "rtsp://admin:7?x@10.0.0.2/x": "rtsp://*:*@10.0.0.2/x",
             "rtsps://u:1/2@h/x": "rtsps://*:*@h/x",
-            "rtsp://admin:pre&token=suf@10.0.0.2/x": "rtsp://*:*@10.0.0.2/x",
-            "rtsp://admin:a?pwd=b@10.0.0.2/x": "rtsp://*:*@10.0.0.2/x",
+            "rtsp://admin:pre&token=suf@10.0.0.2/x": "rtsp://*:*@",
+            "rtsp://admin:a?pwd=b@10.0.0.2/x": "rtsp://*:*@",
+            "rtsp://10.0.0.2:554/live?password=pre@suf": "rtsp://*:*@",
+            "http://10.0.0.2:88/cgi?cmd=snap&usr=admin&pwd=P@ss": "http://*:*@",
+            "ffmpeg:http://h:80/flv?user=u&password=p@ss#video=copy": (
+                "ffmpeg:http://*:*@"
+            ),
+            "rtsp://u:p@10.0.0.2:554/live?token=t": "rtsp://*:*@10.0.0.2:554/live?token=*",
             "http://h/x?api_key=a&apikey=b&access_token=c": (
                 "http://h/x?api_key=*&apikey=*&access_token=*"
             ),

frigate/util/builtin.py

@@ -21,6 +21,7 @@ import numpy as np
 from ruamel.yaml import YAML
 
 from frigate.const import (
+    REGEX_CREDENTIAL_QUERY_KEY,
     REGEX_CREDENTIAL_QUERY_PARAM,
     REGEX_MASKED_CREDENTIAL,
     REGEX_RTSP_CAMERA_USER_PASS,
@@ -121,12 +122,19 @@ def deep_merge(dct1: dict, dct2: dict, override=False, merge_lists=False) -> dic
 def clean_camera_user_pass(line: str) -> str:
     """Mask URL userinfo and credential query parameters in line."""
 
-    # Userinfo goes first so a credential key inside a password can't expose the
-    # rest of it. An "@" in a later query value only over-masks the host.
-    line = re.sub(REGEX_URL_USERINFO, "://*:*@", line)
+    line = re.sub(REGEX_URL_USERINFO, _mask_userinfo, line)
     return re.sub(REGEX_CREDENTIAL_QUERY_PARAM, r"\1*", line)
 
 
+def _mask_userinfo(match: re.Match[str]) -> str:
+    # A credential key in the userinfo means a query password holds "@" or a
+    # password holds a query key. Neither split is safe, so mask the whole URL.
+    if re.search(REGEX_CREDENTIAL_QUERY_KEY, match.group(1)):
+        return "://*:*@"
+
+    return f"://*:*@{match.group(2)}"
+
+
 def is_masked_camera_path(path: str) -> bool:
     """Return True if path holds credentials masked by clean_camera_user_pass."""
     return re.search(REGEX_MASKED_CREDENTIAL, path) is not None

web/src/utils/credentialMask.ts

@@ -2,7 +2,11 @@
 const CREDENTIAL_QUERY_KEYS =
   "(?:user(?:name)?|usr|login|pass(?:word|wd)?|pwd|(?:[a-z_]*_)?token|(?:[a-z_]*_)?secret|client_private|api_?key)";
 const URL_USERINFO_PATTERN =
-  /:\/\/(?:[^\s/?#:[\]]*:(?:(?!:\/\/)\S)*|[^\s/?#]+)@/g;
+  /:\/\/((?:[^\s/?#:[\]]*:(?:(?!:\/\/)\S)*|[^\s/?#]+))@((?:(?!:\/\/)\S)*)/g;
+const CREDENTIAL_QUERY_KEY_PATTERN = new RegExp(
+  `[?&]${CREDENTIAL_QUERY_KEYS}=`,
+  "i",
+);
 const CREDENTIAL_QUERY_PATTERN = new RegExp(
   `([?&]${CREDENTIAL_QUERY_KEYS}=)[^&\\s]*`,
   "gi",
@@ -21,7 +25,11 @@ export const maskCredentials = (value: string): string => {
   }
 
   return value
-    .replace(URL_USERINFO_PATTERN, "://*:*@")
+    .replace(URL_USERINFO_PATTERN, (_match, userinfo: string, rest: string) =>
+      CREDENTIAL_QUERY_KEY_PATTERN.test(userinfo)
+        ? "://*:*@"
+        : `://*:*@${rest}`,
+    )
     .replace(CREDENTIAL_QUERY_PATTERN, "$1*");
 };