diff --git a/src/cryptoadvance/specter/helpers.py b/src/cryptoadvance/specter/helpers.py index b5362fdf2..0dc9037c7 100644 --- a/src/cryptoadvance/specter/helpers.py +++ b/src/cryptoadvance/specter/helpers.py @@ -17,6 +17,7 @@ from embit.transaction import Transaction from embit.liquid.pset import PSET from embit.liquid.transaction import LTransaction from embit.liquid.networks import NETWORKS +from urllib.parse import unquote from .persistence import read_json_file, write_json_file from .util.bcur import bcur_decode import threading @@ -347,4 +348,22 @@ def get_address_from_dict(data_dict): def is_relative_url(url): - return re.match(r"^\/[^\/\\]", url) + # Decode percent-encoded characters so encoded control/whitespace cannot + # bypass validation, e.g. /%09/baidu.com -> "/\t/baidu.com". + normalized = unquote(url) + + if not normalized or not normalized.startswith("/"): + return False + # Reject protocol-relative URLs after decoding. + if normalized.startswith("//"): + return False + # Reject any decoded control characters or whitespace anywhere in the URL, + # including ASCII DEL (0x7f), since browsers may normalize them in unsafe + # ways. + if any(ord(c) < 32 or ord(c) == 127 or c.isspace() for c in normalized): + return False + # Reject backslashes to avoid alternate path separator confusion. + if "\\" in normalized: + return False + + return True diff --git a/tests/test_helpers.py b/tests/test_helpers.py index f508726ea..a1013544c 100644 --- a/tests/test_helpers.py +++ b/tests/test_helpers.py @@ -116,3 +116,44 @@ def test_create_unique_id(): assert not all( create_unique_id(name) == "ghost_wallet" for name in not_the_same_unique_names ) + + +def test_is_relative_url(): + """Test is_relative_url() for open redirect prevention.""" + from cryptoadvance.specter.helpers import is_relative_url + + # Valid relative URLs - should pass + assert is_relative_url("/") + assert is_relative_url("/valid/path") + assert is_relative_url("/path?query=value") + assert is_relative_url("/path#anchor") + assert is_relative_url("/a/b/c/d/e") + + # Invalid - protocol-relative URLs (should be rejected) + assert not is_relative_url("//evil.com") + + # Invalid - percent-encoded bypass attempts + assert not is_relative_url("/%09/baidu.com") # tab -> // bypass + assert not is_relative_url("/%09baidu.com") + assert not is_relative_url("/%5c%5cevil.com") # backslash encoding + assert not is_relative_url("/%20/path") # space + + # Invalid - control characters + assert not is_relative_url("/\x00/path") + assert not is_relative_url("/\x1f/path") + assert not is_relative_url("/\x7f/path") # DEL character + + # Invalid - whitespace + assert not is_relative_url("/ path") + assert not is_relative_url("/path ") # trailing + assert not is_relative_url(" /path") # leading + + # Invalid - backslashes + assert not is_relative_url("/\\path") + assert not is_relative_url("/path\\to\\file") + + # Invalid - non-relative + assert not is_relative_url("") + assert not is_relative_url("https://evil.com") + # /https:// passes validation - it's a local server path, not external + assert is_relative_url("/https://evil.com")