diff --git a/sunnypilot/sunnylink/athena/sunnylinkd.py b/sunnypilot/sunnylink/athena/sunnylinkd.py index 3aeacf6a39..9f70aead59 100755 --- a/sunnypilot/sunnylink/athena/sunnylinkd.py +++ b/sunnypilot/sunnylink/athena/sunnylinkd.py @@ -42,6 +42,12 @@ METADATA_PATH = os.path.join(os.path.dirname(os.path.dirname(os.path.abspath(__f params = Params() +# Parameters that should never be remotely modified for security reasons +BLOCKED_PARAMS = { + "GithubUsername", # Could grant SSH access + "GithubSshKeys", # Direct SSH key injection +} + def handle_long_poll(ws: WebSocket, exit_event: threading.Event | None) -> None: cloudlog.info("sunnylinkd.handle_long_poll started") @@ -248,6 +254,11 @@ def getParams(params_keys: list[str], compression: bool = False) -> str | dict[s @dispatcher.add_method def saveParams(params_to_update: dict[str, str], compression: bool = False) -> None: for key, value in params_to_update.items(): + # disallow modifications to blocked parameters + if key in BLOCKED_PARAMS: + cloudlog.warning(f"sunnylinkd.saveParams.blocked: Attempted to modify blocked parameter '{key}'") + continue + try: save_param_from_base64_encoded_string(key, value, compression) except Exception as e: diff --git a/sunnypilot/sunnylink/athena/tests/test_sunnylinkd.py b/sunnypilot/sunnylink/athena/tests/test_sunnylinkd.py new file mode 100644 index 0000000000..616bff037e --- /dev/null +++ b/sunnypilot/sunnylink/athena/tests/test_sunnylinkd.py @@ -0,0 +1,59 @@ +""" +Copyright (c) 2021-, Haibin Wen, sunnypilot, and a number of other contributors. + +This file is part of sunnypilot and is licensed under the MIT License. +See the LICENSE.md file in the root directory for more details. +""" +from openpilot.sunnypilot.sunnylink.athena import sunnylinkd + + +class TestSunnylinkdMethods: + def setup_method(self): + self.saved_params = [] + + self.original_save = sunnylinkd.save_param_from_base64_encoded_string + + def mock_save_param(key, value, compression=False): + self.saved_params.append((key, value, compression)) + + sunnylinkd.save_param_from_base64_encoded_string = mock_save_param + + def teardown_method(self): + sunnylinkd.save_param_from_base64_encoded_string = self.original_save + + def test_saveParams_blocked(self): + blocked_params = { + "GithubUsername": "attacker", + "GithubSshKeys": "ssh-rsa attacker_key", + } + + sunnylinkd.saveParams(blocked_params) + + assert len(self.saved_params) == 0 + + def test_saveParams_allowed(self): + allowed_params = { + "SpeedLimitOffset": "5", + "MyCustomParam": "123" + } + + sunnylinkd.saveParams(allowed_params) + + # verify content + assert len(self.saved_params) == 2 + keys_saved = [p[0] for p in self.saved_params] + assert "SpeedLimitOffset" in keys_saved + assert "MyCustomParam" in keys_saved + + def test_saveParams_mixed(self): + mixed_params = { + "GithubUsername": "attacker", + "SpeedLimitOffset": "10" + } + + sunnylinkd.saveParams(mixed_params) + + # should save allowed one + assert len(self.saved_params) == 1 + assert self.saved_params[0][0] == "SpeedLimitOffset" + assert self.saved_params[0][1] == "10"