From 9616596146395191bfcec01cc310b0cef58d1c1d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 06:40:30 -0600 Subject: [PATCH 01/30] [ota] Use secrets module for OTA authentication cnonce Replace random.random() with secrets.token_hex() for generating the client nonce in OTA challenge-response authentication. The random module uses Mersenne Twister which is not cryptographically secure. The secrets module is the correct choice for security-sensitive token generation. --- esphome/espota2.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/esphome/espota2.py b/esphome/espota2.py index 2d90251b38..bdfa7cb242 100644 --- a/esphome/espota2.py +++ b/esphome/espota2.py @@ -6,7 +6,7 @@ import hashlib import io import logging from pathlib import Path -import random +import secrets import socket import sys import time @@ -301,7 +301,7 @@ def perform_ota( _LOGGER.debug("Auth: %s Nonce is %s", hash_name, nonce) # Generate cnonce - cnonce = hash_func(str(random.random()).encode()).hexdigest() + cnonce = secrets.token_hex(32) _LOGGER.debug("Auth: %s CNonce is %s", hash_name, cnonce) send_check(sock, cnonce, "auth cnonce") From e0396764223377c0bbd0f6bb2a2503bb7a52e0a3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 06:44:45 -0600 Subject: [PATCH 02/30] [wizard] Use secrets module for fallback AP password generation Replace random.choice() with secrets.choice() for generating the fallback hotspot password. The random module uses Mersenne Twister which is not cryptographically secure. The secrets module is the correct choice for credential generation. The file already imports secrets for other credential generation. --- esphome/wizard.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/esphome/wizard.py b/esphome/wizard.py index 4b74847996..f83342cc6a 100644 --- a/esphome/wizard.py +++ b/esphome/wizard.py @@ -1,6 +1,5 @@ import base64 from pathlib import Path -import random import secrets import string from typing import Literal, NotRequired, TypedDict, Unpack @@ -130,7 +129,7 @@ def wizard_file(**kwargs: Unpack[WizardFileKwargs]) -> str: if len(ap_name) > 32: ap_name = ap_name_base kwargs["fallback_name"] = ap_name - kwargs["fallback_psk"] = "".join(random.choice(letters) for _ in range(12)) + kwargs["fallback_psk"] = "".join(secrets.choice(letters) for _ in range(12)) base = BASE_CONFIG_FRIENDLY if kwargs.get("friendly_name") else BASE_CONFIG From 79a205eee27ac37e344fc0b35406bd118a43605f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 06:46:28 -0600 Subject: [PATCH 03/30] [dashboard] Use constant-time comparison for username check Use hmac.compare_digest() for the username comparison to match the existing constant-time password comparison. This prevents username enumeration via timing analysis. --- esphome/dashboard/settings.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/esphome/dashboard/settings.py b/esphome/dashboard/settings.py index 6035b4a1d6..5baa03d02d 100644 --- a/esphome/dashboard/settings.py +++ b/esphome/dashboard/settings.py @@ -84,11 +84,12 @@ class DashboardSettings: def check_password(self, username: str, password: str) -> bool: if not self.using_auth: return True - if username != self.username: - return False - - # Compare password in constant running time (to prevent timing attacks) - return hmac.compare_digest(self.password_hash, password_hash(password)) + # Compare both in constant running time (to prevent timing attacks) + username_matches = hmac.compare_digest(username, self.username) + password_matches = hmac.compare_digest( + self.password_hash, password_hash(password) + ) + return username_matches and password_matches def rel_path(self, *args: Any) -> Path: """Return a path relative to the ESPHome config folder.""" From 2829f7b4859181c91016e4a757bb8ea23396f075 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 06:47:16 -0600 Subject: [PATCH 04/30] [dashboard] Handle malformed Basic Auth headers gracefully Wrap base64 decode and split in try/except so malformed Authorization headers return a clean 401 instead of an unhandled exception producing a 500 response with stack trace in logs. Catches ValueError (covers binascii.Error from b64decode) and UnicodeDecodeError (from .decode()). --- esphome/dashboard/web_server.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/esphome/dashboard/web_server.py b/esphome/dashboard/web_server.py index da50279864..52e16e1ad7 100644 --- a/esphome/dashboard/web_server.py +++ b/esphome/dashboard/web_server.py @@ -120,8 +120,11 @@ def is_authenticated(handler: BaseHandler) -> bool: if auth_header := handler.request.headers.get("Authorization"): assert isinstance(auth_header, str) if auth_header.startswith("Basic "): - auth_decoded = base64.b64decode(auth_header[6:]).decode() - username, password = auth_decoded.split(":", 1) + try: + auth_decoded = base64.b64decode(auth_header[6:]).decode() + username, password = auth_decoded.split(":", 1) + except (ValueError, UnicodeDecodeError): + return False return settings.check_password(username, password) return handler.get_secure_cookie(AUTH_COOKIE_NAME) == COOKIE_AUTHENTICATED_YES From a40c87eeedc85414e8023aceb5a37cd46788a318 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 06:48:38 -0600 Subject: [PATCH 05/30] [dashboard] Use resolve/relative_to for download path validation Replace string-based path sanitization (.replace/.lstrip) with Path.resolve() and relative_to() validation, matching the pattern used by other dashboard endpoints (e.g. settings.rel_path). The previous approach was not exploitable but was inconsistent with the rest of the codebase. --- esphome/dashboard/web_server.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/esphome/dashboard/web_server.py b/esphome/dashboard/web_server.py index da50279864..24a0bacf05 100644 --- a/esphome/dashboard/web_server.py +++ b/esphome/dashboard/web_server.py @@ -1057,14 +1057,19 @@ class DownloadBinaryRequestHandler(BaseHandler): if file_name is None: self.send_error(400) return - file_name = file_name.replace("..", "").lstrip("/") # get requested download name, or build it based on filename download_name = self.get_argument( "download", f"{storage_json.name}-{file_name}", ) - path = storage_json.firmware_bin_path.parent.joinpath(file_name) + base_dir = storage_json.firmware_bin_path.parent.resolve() + path = base_dir.joinpath(file_name).resolve() + try: + path.relative_to(base_dir) + except ValueError: + self.send_error(403) + return if not path.is_file(): args = ["esphome", "idedata", settings.rel_path(configuration)] From 1dcffdc872cbc9b051d0a5f8ba1e6f61f2afea44 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 06:51:52 -0600 Subject: [PATCH 06/30] [web_server_idf] Use constant-time comparison for Basic Auth Replace strcmp() with a constant-time XOR accumulation loop for comparing base64-encoded credentials in HTTP Basic Auth. --- .../components/web_server_idf/web_server_idf.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/esphome/components/web_server_idf/web_server_idf.cpp b/esphome/components/web_server_idf/web_server_idf.cpp index 9860810452..074c39a6ae 100644 --- a/esphome/components/web_server_idf/web_server_idf.cpp +++ b/esphome/components/web_server_idf/web_server_idf.cpp @@ -354,7 +354,18 @@ bool AsyncWebServerRequest::authenticate(const char *username, const char *passw esp_crypto_base64_encode(reinterpret_cast(digest.get()), n, &out, reinterpret_cast(user_info), user_info_len); - return strcmp(digest.get(), auth_str + auth_prefix_len) == 0; + // Constant-time comparison to avoid timing side channels + const char *provided = auth_str + auth_prefix_len; + size_t digest_len = strlen(digest.get()); + size_t provided_len = strlen(provided); + if (digest_len != provided_len) { + return false; + } + volatile uint8_t result = 0; + for (size_t i = 0; i < digest_len; i++) { + result |= digest.get()[i] ^ provided[i]; + } + return result == 0; } void AsyncWebServerRequest::requestAuthentication(const char *realm) const { From a8fd6c132eb69c67504dda1ce9a3f3bf5e6a8694 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 06:59:32 -0600 Subject: [PATCH 07/30] Update tests to mock secrets.token_hex instead of random.random The cnonce generation was changed to use secrets.token_hex(32), so the test mocks and assertions need to match. --- tests/unit_tests/test_espota2.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/unit_tests/test_espota2.py b/tests/unit_tests/test_espota2.py index 1885b769f1..209fe81065 100644 --- a/tests/unit_tests/test_espota2.py +++ b/tests/unit_tests/test_espota2.py @@ -18,8 +18,7 @@ from esphome import espota2 from esphome.core import EsphomeError # Test constants -MOCK_RANDOM_VALUE = 0.123456 -MOCK_RANDOM_BYTES = b"0.123456" +MOCK_CNONCE = "a" * 64 # Mock 64-char hex string from secrets.token_hex(32) MOCK_MD5_NONCE = b"12345678901234567890123456789012" # 32 char nonce for MD5 MOCK_SHA256_NONCE = b"1234567890123456789012345678901234567890123456789012345678901234" # 64 char nonce for SHA256 @@ -56,8 +55,10 @@ def mock_time() -> Generator[None]: @pytest.fixture def mock_random() -> Generator[Mock]: - """Mock random for predictable test values.""" - with patch("random.random", return_value=MOCK_RANDOM_VALUE) as mock_rand: + """Mock secrets.token_hex for predictable test values.""" + with patch( + "esphome.espota2.secrets.token_hex", return_value=MOCK_CNONCE + ) as mock_rand: yield mock_rand @@ -272,8 +273,8 @@ def test_perform_ota_successful_md5_auth( ) ) - # Verify cnonce was sent (MD5 of random.random()) - cnonce = hashlib.md5(MOCK_RANDOM_BYTES).hexdigest() + # Verify cnonce was sent + cnonce = MOCK_CNONCE assert mock_socket.sendall.call_args_list[2] == call(cnonce.encode()) # Verify auth result was computed correctly @@ -639,8 +640,8 @@ def test_perform_ota_successful_sha256_auth( ) ) - # Verify cnonce was sent (SHA256 of random.random()) - cnonce = hashlib.sha256(MOCK_RANDOM_BYTES).hexdigest() + # Verify cnonce was sent + cnonce = MOCK_CNONCE assert mock_socket.sendall.call_args_list[2] == call(cnonce.encode()) # Verify auth result was computed correctly with SHA256 @@ -692,7 +693,7 @@ def test_perform_ota_sha256_fallback_to_md5( ) # But authentication was done with MD5 - cnonce = hashlib.md5(MOCK_RANDOM_BYTES).hexdigest() + cnonce = MOCK_CNONCE expected_hash = hashlib.md5() expected_hash.update(b"testpass") expected_hash.update(MOCK_MD5_NONCE) From 803b9a7a181b099ed7fb8baa31c028e79b2b79a9 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:01:37 -0600 Subject: [PATCH 08/30] Update path traversal tests for resolve/relative_to behavior Real traversals that escape the base directory now return 403. Paths like '....' that resolve inside the base directory but don't exist return 404. --- tests/dashboard/test_web_server.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/tests/dashboard/test_web_server.py b/tests/dashboard/test_web_server.py index 7642876ee5..4b06bfd7c6 100644 --- a/tests/dashboard/test_web_server.py +++ b/tests/dashboard/test_web_server.py @@ -528,14 +528,14 @@ async def test_download_binary_handler_subdirectory_file_url_encoded( @pytest.mark.asyncio @pytest.mark.usefixtures("mock_ext_storage_path") @pytest.mark.parametrize( - "attack_path", + ("attack_path", "expected_code"), [ - pytest.param("../../../secrets.yaml", id="basic_traversal"), - pytest.param("..%2F..%2F..%2Fsecrets.yaml", id="url_encoded"), - pytest.param("zephyr/../../../secrets.yaml", id="traversal_with_prefix"), - pytest.param("/etc/passwd", id="absolute_path"), - pytest.param("//etc/passwd", id="double_slash_absolute"), - pytest.param("....//secrets.yaml", id="multiple_dots"), + pytest.param("../../../secrets.yaml", 403, id="basic_traversal"), + pytest.param("..%2F..%2F..%2Fsecrets.yaml", 403, id="url_encoded"), + pytest.param("zephyr/../../../secrets.yaml", 403, id="traversal_with_prefix"), + pytest.param("/etc/passwd", 403, id="absolute_path"), + pytest.param("//etc/passwd", 403, id="double_slash_absolute"), + pytest.param("....//secrets.yaml", 404, id="multiple_dots"), ], ) async def test_download_binary_handler_path_traversal_protection( @@ -543,11 +543,14 @@ async def test_download_binary_handler_path_traversal_protection( tmp_path: Path, mock_storage_json: MagicMock, attack_path: str, + expected_code: int, ) -> None: """Test that DownloadBinaryRequestHandler prevents path traversal attacks. Verifies that attempts to use '..' in file paths are sanitized to prevent accessing files outside the build directory. Tests multiple attack vectors. + Real traversals that escape the base directory get 403. Paths like '....' + that resolve inside the base directory but don't exist get 404. """ # Create build structure build_dir = get_build_path(tmp_path, "test") @@ -571,8 +574,7 @@ async def test_download_binary_handler_path_traversal_protection( f"/download.bin?configuration=test.yaml&file={attack_path}", method="GET", ) - # Should get 404 (file not found after sanitization) or 500 (idedata fails) - assert exc_info.value.code in (404, 500) + assert exc_info.value.code == expected_code @pytest.mark.asyncio From 42126bae72b6ebff0adeb5e351a2943fd37a9cf1 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:03:29 -0600 Subject: [PATCH 09/30] Add test coverage for check_password Tests correct credentials, wrong password, wrong username, both wrong, and auth-disabled cases. --- tests/dashboard/test_settings.py | 39 ++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tests/dashboard/test_settings.py b/tests/dashboard/test_settings.py index 91a8ec70c3..32e581083a 100644 --- a/tests/dashboard/test_settings.py +++ b/tests/dashboard/test_settings.py @@ -10,6 +10,7 @@ import pytest from esphome.core import CORE from esphome.dashboard.settings import DashboardSettings +from esphome.dashboard.util.password import password_hash @pytest.fixture @@ -221,3 +222,41 @@ def test_config_path_parent_resolves_to_config_dir(tmp_path: Path) -> None: # Verify that CORE.config_path itself uses the sentinel file assert CORE.config_path.name == "___DASHBOARD_SENTINEL___.yaml" assert not CORE.config_path.exists() # Sentinel file doesn't actually exist + + +@pytest.fixture +def auth_settings(tmp_path: Path) -> DashboardSettings: + """Create DashboardSettings with auth configured.""" + settings = DashboardSettings() + resolved_dir = tmp_path.resolve() + settings.config_dir = resolved_dir + settings.absolute_config_dir = resolved_dir + settings.username = "admin" + settings.using_password = True + settings.password_hash = password_hash("correctpassword") + return settings + + +def test_check_password_correct_credentials(auth_settings: DashboardSettings) -> None: + """Test check_password returns True for correct username and password.""" + assert auth_settings.check_password("admin", "correctpassword") is True + + +def test_check_password_wrong_password(auth_settings: DashboardSettings) -> None: + """Test check_password returns False for wrong password.""" + assert auth_settings.check_password("admin", "wrongpassword") is False + + +def test_check_password_wrong_username(auth_settings: DashboardSettings) -> None: + """Test check_password returns False for wrong username.""" + assert auth_settings.check_password("notadmin", "correctpassword") is False + + +def test_check_password_both_wrong(auth_settings: DashboardSettings) -> None: + """Test check_password returns False when both are wrong.""" + assert auth_settings.check_password("notadmin", "wrongpassword") is False + + +def test_check_password_no_auth(dashboard_settings: DashboardSettings) -> None: + """Test check_password returns True when auth is not configured.""" + assert dashboard_settings.check_password("anyone", "anything") is True From 806a86a6ada3b13f90d88452aa85c26995dd10a8 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:06:24 -0600 Subject: [PATCH 10/30] Add test coverage for is_authenticated base64 handling Tests malformed base64, invalid UTF-8, missing colon separator, valid credentials, wrong credentials, and auth-disabled cases. --- tests/dashboard/test_web_server.py | 79 ++++++++++++++++++++++++++++++ 1 file changed, 79 insertions(+) diff --git a/tests/dashboard/test_web_server.py b/tests/dashboard/test_web_server.py index 7642876ee5..985efa3157 100644 --- a/tests/dashboard/test_web_server.py +++ b/tests/dashboard/test_web_server.py @@ -2,6 +2,7 @@ from __future__ import annotations from argparse import Namespace import asyncio +import base64 from collections.abc import Generator from contextlib import asynccontextmanager import gzip @@ -1676,3 +1677,81 @@ def test_proc_on_exit_skips_when_already_closed() -> None: handler.write_message.assert_not_called() handler.close.assert_not_called() + + +def _make_auth_handler(auth_header: str | None = None) -> Mock: + """Create a mock handler with the given Authorization header.""" + handler = Mock() + handler.request = Mock() + if auth_header is not None: + handler.request.headers = {"Authorization": auth_header} + else: + handler.request.headers = {} + handler.get_secure_cookie = Mock(return_value=None) + return handler + + +@pytest.fixture +def mock_auth_settings(mock_dashboard_settings: MagicMock) -> MagicMock: + """Fixture to configure mock dashboard settings with auth enabled.""" + mock_dashboard_settings.using_auth = True + mock_dashboard_settings.on_ha_addon = False + return mock_dashboard_settings + + +def test_is_authenticated_malformed_base64( + mock_auth_settings: MagicMock, +) -> None: + """Test that invalid base64 in Authorization header returns False.""" + handler = _make_auth_handler("Basic !!!not-valid-base64!!!") + assert web_server.is_authenticated(handler) is False + + +def test_is_authenticated_invalid_utf8( + mock_auth_settings: MagicMock, +) -> None: + """Test that base64 decoding to invalid UTF-8 returns False.""" + # \xff\xfe is invalid UTF-8 + bad_payload = base64.b64encode(b"\xff\xfe").decode("ascii") + handler = _make_auth_handler(f"Basic {bad_payload}") + assert web_server.is_authenticated(handler) is False + + +def test_is_authenticated_no_colon( + mock_auth_settings: MagicMock, +) -> None: + """Test that base64 payload without ':' separator returns False.""" + no_colon = base64.b64encode(b"nocolonhere").decode("ascii") + handler = _make_auth_handler(f"Basic {no_colon}") + assert web_server.is_authenticated(handler) is False + + +def test_is_authenticated_valid_credentials( + mock_auth_settings: MagicMock, +) -> None: + """Test that valid Basic auth credentials are checked.""" + creds = base64.b64encode(b"admin:secret").decode("ascii") + mock_auth_settings.check_password.return_value = True + handler = _make_auth_handler(f"Basic {creds}") + assert web_server.is_authenticated(handler) is True + mock_auth_settings.check_password.assert_called_once_with("admin", "secret") + + +def test_is_authenticated_wrong_credentials( + mock_auth_settings: MagicMock, +) -> None: + """Test that valid Basic auth with wrong credentials returns False.""" + creds = base64.b64encode(b"admin:wrong").decode("ascii") + mock_auth_settings.check_password.return_value = False + handler = _make_auth_handler(f"Basic {creds}") + assert web_server.is_authenticated(handler) is False + + +def test_is_authenticated_no_auth_configured( + mock_dashboard_settings: MagicMock, +) -> None: + """Test that requests pass when auth is not configured.""" + mock_dashboard_settings.using_auth = False + mock_dashboard_settings.on_ha_addon = False + handler = _make_auth_handler() + assert web_server.is_authenticated(handler) is True From caff93d7b8d4bcf70b7dab8642c89f13b812fb5a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:08:25 -0600 Subject: [PATCH 11/30] Add test coverage for secrets.choice in fallback PSK generation Verifies that wizard_file uses secrets.choice (not random.choice) to generate the 12-character fallback hotspot password. --- tests/unit_tests/test_wizard.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/unit_tests/test_wizard.py b/tests/unit_tests/test_wizard.py index eb44c1c20f..0ce89230d8 100644 --- a/tests/unit_tests/test_wizard.py +++ b/tests/unit_tests/test_wizard.py @@ -2,7 +2,7 @@ from pathlib import Path from typing import Any -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch import pytest from pytest import MonkeyPatch @@ -632,3 +632,14 @@ def test_wizard_accepts_rpipico_board(tmp_path: Path, monkeypatch: MonkeyPatch): # rpipico doesn't support WiFi, so no api_encryption_key or ota_password assert "api_encryption_key" not in call_kwargs assert "ota_password" not in call_kwargs + + +def test_fallback_psk_uses_secrets_choice( + default_config: dict[str, Any], +) -> None: + """Test that fallback PSK is generated using secrets.choice.""" + with patch("esphome.wizard.secrets.choice", return_value="X") as mock_choice: + config = wz.wizard_file(**default_config) + + assert 'password: "XXXXXXXXXXXX"' in config + assert mock_choice.call_count == 12 From 1b7efdd0519ca441ecea0c334b5cd76158f701d0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:11:56 -0600 Subject: [PATCH 12/30] Match cnonce length to hash algorithm digest size Use nonce_size // 2 as token_hex argument so MD5 auth produces a 32-char cnonce and SHA256 auth produces a 64-char cnonce, matching the original protocol behavior. Rename mock_random fixture to mock_token_hex and use separate mock cnonce constants per hash algorithm. --- esphome/espota2.py | 4 ++-- tests/unit_tests/test_espota2.py | 40 +++++++++++++++++++++----------- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/esphome/espota2.py b/esphome/espota2.py index bdfa7cb242..c342eb4463 100644 --- a/esphome/espota2.py +++ b/esphome/espota2.py @@ -300,8 +300,8 @@ def perform_ota( nonce = nonce_bytes.decode() _LOGGER.debug("Auth: %s Nonce is %s", hash_name, nonce) - # Generate cnonce - cnonce = secrets.token_hex(32) + # Generate cnonce matching the hash algorithm's digest size + cnonce = secrets.token_hex(nonce_size // 2) _LOGGER.debug("Auth: %s CNonce is %s", hash_name, cnonce) send_check(sock, cnonce, "auth cnonce") diff --git a/tests/unit_tests/test_espota2.py b/tests/unit_tests/test_espota2.py index 209fe81065..57e33e083e 100644 --- a/tests/unit_tests/test_espota2.py +++ b/tests/unit_tests/test_espota2.py @@ -18,7 +18,8 @@ from esphome import espota2 from esphome.core import EsphomeError # Test constants -MOCK_CNONCE = "a" * 64 # Mock 64-char hex string from secrets.token_hex(32) +MOCK_MD5_CNONCE = "a" * 32 # Mock 32-char hex string from secrets.token_hex(16) +MOCK_SHA256_CNONCE = "b" * 64 # Mock 64-char hex string from secrets.token_hex(32) MOCK_MD5_NONCE = b"12345678901234567890123456789012" # 32 char nonce for MD5 MOCK_SHA256_NONCE = b"1234567890123456789012345678901234567890123456789012345678901234" # 64 char nonce for SHA256 @@ -54,12 +55,16 @@ def mock_time() -> Generator[None]: @pytest.fixture -def mock_random() -> Generator[Mock]: +def mock_token_hex() -> Generator[Mock]: """Mock secrets.token_hex for predictable test values.""" - with patch( - "esphome.espota2.secrets.token_hex", return_value=MOCK_CNONCE - ) as mock_rand: - yield mock_rand + + def _token_hex(nbytes: int) -> str: + if nbytes == 16: + return MOCK_MD5_CNONCE + return MOCK_SHA256_CNONCE + + with patch("esphome.espota2.secrets.token_hex", side_effect=_token_hex) as mock: + yield mock @pytest.fixture @@ -237,7 +242,7 @@ def test_send_check_socket_error(mock_socket: Mock) -> None: @pytest.mark.usefixtures("mock_time") def test_perform_ota_successful_md5_auth( - mock_socket: Mock, mock_file: io.BytesIO, mock_random: Mock + mock_socket: Mock, mock_file: io.BytesIO, mock_token_hex: Mock ) -> None: """Test successful OTA with MD5 authentication.""" # Setup socket responses for recv calls @@ -273,8 +278,11 @@ def test_perform_ota_successful_md5_auth( ) ) + # Verify token_hex was called with MD5 digest size + mock_token_hex.assert_called_once_with(16) + # Verify cnonce was sent - cnonce = MOCK_CNONCE + cnonce = MOCK_MD5_CNONCE assert mock_socket.sendall.call_args_list[2] == call(cnonce.encode()) # Verify auth result was computed correctly @@ -367,7 +375,7 @@ def test_perform_ota_auth_without_password(mock_socket: Mock) -> None: @pytest.mark.usefixtures("mock_time") def test_perform_ota_md5_auth_wrong_password( - mock_socket: Mock, mock_file: io.BytesIO, mock_random: Mock + mock_socket: Mock, mock_file: io.BytesIO, mock_token_hex: Mock ) -> None: """Test OTA fails when MD5 authentication is rejected due to wrong password.""" # Setup socket responses for recv calls @@ -391,7 +399,7 @@ def test_perform_ota_md5_auth_wrong_password( @pytest.mark.usefixtures("mock_time") def test_perform_ota_sha256_auth_wrong_password( - mock_socket: Mock, mock_file: io.BytesIO, mock_random: Mock + mock_socket: Mock, mock_file: io.BytesIO, mock_token_hex: Mock ) -> None: """Test OTA fails when SHA256 authentication is rejected due to wrong password.""" # Setup socket responses for recv calls @@ -604,7 +612,7 @@ def test_progress_bar(capsys: CaptureFixture[str]) -> None: # Tests for SHA256 authentication @pytest.mark.usefixtures("mock_time") def test_perform_ota_successful_sha256_auth( - mock_socket: Mock, mock_file: io.BytesIO, mock_random: Mock + mock_socket: Mock, mock_file: io.BytesIO, mock_token_hex: Mock ) -> None: """Test successful OTA with SHA256 authentication.""" # Setup socket responses for recv calls @@ -640,8 +648,11 @@ def test_perform_ota_successful_sha256_auth( ) ) + # Verify token_hex was called with SHA256 digest size + mock_token_hex.assert_called_once_with(32) + # Verify cnonce was sent - cnonce = MOCK_CNONCE + cnonce = MOCK_SHA256_CNONCE assert mock_socket.sendall.call_args_list[2] == call(cnonce.encode()) # Verify auth result was computed correctly with SHA256 @@ -655,7 +666,7 @@ def test_perform_ota_successful_sha256_auth( @pytest.mark.usefixtures("mock_time") def test_perform_ota_sha256_fallback_to_md5( - mock_socket: Mock, mock_file: io.BytesIO, mock_random: Mock + mock_socket: Mock, mock_file: io.BytesIO, mock_token_hex: Mock ) -> None: """Test SHA256-capable client falls back to MD5 for compatibility.""" # This test verifies the temporary backward compatibility @@ -693,7 +704,8 @@ def test_perform_ota_sha256_fallback_to_md5( ) # But authentication was done with MD5 - cnonce = MOCK_CNONCE + mock_token_hex.assert_called_once_with(16) + cnonce = MOCK_MD5_CNONCE expected_hash = hashlib.md5() expected_hash.update(b"testpass") expected_hash.update(MOCK_MD5_NONCE) From a167332518a0c7bb37c59d3a5b35fc9021557926 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:14:20 -0600 Subject: [PATCH 13/30] Fix password_hash type and add HA add-on regression test Initialize password_hash as b"" (bytes) to match password_hash() return type, preventing TypeError in hmac.compare_digest when HA add-on auth is enabled without a password. --- esphome/dashboard/settings.py | 4 ++-- tests/dashboard/test_settings.py | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/esphome/dashboard/settings.py b/esphome/dashboard/settings.py index 5baa03d02d..aa994ae487 100644 --- a/esphome/dashboard/settings.py +++ b/esphome/dashboard/settings.py @@ -32,7 +32,7 @@ class DashboardSettings: def __init__(self) -> None: """Initialize the dashboard settings.""" self.config_dir: Path = None - self.password_hash: str = "" + self.password_hash: bytes = b"" self.username: str = "" self.using_password: bool = False self.on_ha_addon: bool = False @@ -84,7 +84,7 @@ class DashboardSettings: def check_password(self, username: str, password: str) -> bool: if not self.using_auth: return True - # Compare both in constant running time (to prevent timing attacks) + # Compare in constant running time (to prevent timing attacks) username_matches = hmac.compare_digest(username, self.username) password_matches = hmac.compare_digest( self.password_hash, password_hash(password) diff --git a/tests/dashboard/test_settings.py b/tests/dashboard/test_settings.py index 32e581083a..a7594e4222 100644 --- a/tests/dashboard/test_settings.py +++ b/tests/dashboard/test_settings.py @@ -260,3 +260,18 @@ def test_check_password_both_wrong(auth_settings: DashboardSettings) -> None: def test_check_password_no_auth(dashboard_settings: DashboardSettings) -> None: """Test check_password returns True when auth is not configured.""" assert dashboard_settings.check_password("anyone", "anything") is True + + +def test_check_password_ha_addon_no_password( + dashboard_settings: DashboardSettings, +) -> None: + """Test check_password doesn't crash in HA add-on mode without a password. + + In HA add-on mode, using_ha_addon_auth can be True while using_password + is False, leaving password_hash as b"". This must not raise TypeError + in hmac.compare_digest. + """ + dashboard_settings.on_ha_addon = True + dashboard_settings.using_password = False + # password_hash stays as default b"" + assert dashboard_settings.check_password("anyone", "anything") is False From c90ca4df8768d18f949f09e3d3c8bf50d81379e3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:16:19 -0600 Subject: [PATCH 14/30] Use encoder output length and remove early return on length mismatch Use the out length from esp_crypto_base64_encode instead of strlen. Fold length mismatch into the accumulator to make comparison fully constant-time without early return. --- esphome/components/web_server_idf/web_server_idf.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/esphome/components/web_server_idf/web_server_idf.cpp b/esphome/components/web_server_idf/web_server_idf.cpp index 074c39a6ae..2c2ceef3a6 100644 --- a/esphome/components/web_server_idf/web_server_idf.cpp +++ b/esphome/components/web_server_idf/web_server_idf.cpp @@ -356,14 +356,13 @@ bool AsyncWebServerRequest::authenticate(const char *username, const char *passw // Constant-time comparison to avoid timing side channels const char *provided = auth_str + auth_prefix_len; - size_t digest_len = strlen(digest.get()); + size_t digest_len = out; size_t provided_len = strlen(provided); - if (digest_len != provided_len) { - return false; - } volatile uint8_t result = 0; + result |= static_cast(digest_len ^ provided_len); for (size_t i = 0; i < digest_len; i++) { - result |= digest.get()[i] ^ provided[i]; + char provided_ch = (i < provided_len) ? provided[i] : 0; + result |= static_cast(digest.get()[i] ^ provided_ch); } return result == 0; } From 999774889d5ba2546223eb037912d33f64b3d11f Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:17:24 -0600 Subject: [PATCH 15/30] Add comments explaining constant-time comparison logic --- esphome/components/web_server_idf/web_server_idf.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/esphome/components/web_server_idf/web_server_idf.cpp b/esphome/components/web_server_idf/web_server_idf.cpp index 2c2ceef3a6..7cb56def57 100644 --- a/esphome/components/web_server_idf/web_server_idf.cpp +++ b/esphome/components/web_server_idf/web_server_idf.cpp @@ -354,13 +354,19 @@ bool AsyncWebServerRequest::authenticate(const char *username, const char *passw esp_crypto_base64_encode(reinterpret_cast(digest.get()), n, &out, reinterpret_cast(user_info), user_info_len); - // Constant-time comparison to avoid timing side channels + // Constant-time comparison to avoid timing side channels. + // No early return on length mismatch — the length difference is folded + // into the accumulator so the loop always runs over the full digest. const char *provided = auth_str + auth_prefix_len; - size_t digest_len = out; + size_t digest_len = out; // length from esp_crypto_base64_encode size_t provided_len = strlen(provided); volatile uint8_t result = 0; + // Non-zero if lengths differ; XOR of two size_t values truncated to 8 bits + // catches differences in the low byte, and any length mismatch also causes + // the byte-wise loop below to accumulate non-zero XOR values. result |= static_cast(digest_len ^ provided_len); for (size_t i = 0; i < digest_len; i++) { + // Bounds-safe: use 0 for bytes beyond provided_len char provided_ch = (i < provided_len) ? provided[i] : 0; result |= static_cast(digest.get()[i] ^ provided_ch); } From 82d9616f1b42c3f24c1a9eda64c774d248c0bde1 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:18:29 -0600 Subject: [PATCH 16/30] Add explicit binascii.Error catch and bad-padding test binascii.Error is already a subclass of ValueError, but listing it explicitly makes the intent clear. Added test for incorrect base64 padding (e.g. "Basic abc"). --- esphome/dashboard/web_server.py | 2 +- tests/dashboard/test_web_server.py | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/esphome/dashboard/web_server.py b/esphome/dashboard/web_server.py index 52e16e1ad7..3f29e54d24 100644 --- a/esphome/dashboard/web_server.py +++ b/esphome/dashboard/web_server.py @@ -123,7 +123,7 @@ def is_authenticated(handler: BaseHandler) -> bool: try: auth_decoded = base64.b64decode(auth_header[6:]).decode() username, password = auth_decoded.split(":", 1) - except (ValueError, UnicodeDecodeError): + except (binascii.Error, ValueError, UnicodeDecodeError): return False return settings.check_password(username, password) return handler.get_secure_cookie(AUTH_COOKIE_NAME) == COOKIE_AUTHENTICATED_YES diff --git a/tests/dashboard/test_web_server.py b/tests/dashboard/test_web_server.py index 985efa3157..66497088fb 100644 --- a/tests/dashboard/test_web_server.py +++ b/tests/dashboard/test_web_server.py @@ -1707,6 +1707,14 @@ def test_is_authenticated_malformed_base64( assert web_server.is_authenticated(handler) is False +def test_is_authenticated_bad_base64_padding( + mock_auth_settings: MagicMock, +) -> None: + """Test that incorrect base64 padding (binascii.Error) returns False.""" + handler = _make_auth_handler("Basic abc") + assert web_server.is_authenticated(handler) is False + + def test_is_authenticated_invalid_utf8( mock_auth_settings: MagicMock, ) -> None: From e362e6fe2f571cd0587482454b16783a67b2e7b9 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:22:08 -0600 Subject: [PATCH 17/30] Fix multiple_dots test for Windows path resolution On Windows, ....//secrets.yaml escapes the base directory (403), while on Unix it stays inside (404). Use sys.platform to set the expected status code per platform. --- tests/dashboard/test_web_server.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/dashboard/test_web_server.py b/tests/dashboard/test_web_server.py index 4b06bfd7c6..d216652162 100644 --- a/tests/dashboard/test_web_server.py +++ b/tests/dashboard/test_web_server.py @@ -8,6 +8,7 @@ import gzip import json import os from pathlib import Path +import sys from unittest.mock import AsyncMock, MagicMock, Mock, patch import pytest @@ -535,7 +536,11 @@ async def test_download_binary_handler_subdirectory_file_url_encoded( pytest.param("zephyr/../../../secrets.yaml", 403, id="traversal_with_prefix"), pytest.param("/etc/passwd", 403, id="absolute_path"), pytest.param("//etc/passwd", 403, id="double_slash_absolute"), - pytest.param("....//secrets.yaml", 404, id="multiple_dots"), + pytest.param( + "....//secrets.yaml", + 403 if sys.platform == "win32" else 404, + id="multiple_dots", + ), ], ) async def test_download_binary_handler_path_traversal_protection( From 43448d55f10734af714685038e049f9bc30ce0c3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:23:24 -0600 Subject: [PATCH 18/30] Guard against None firmware_bin_path and mock subprocess in tests - Add None check for storage_json.firmware_bin_path before computing base_dir (covers configs from StorageJSON.from_wizard()). - Mock async_run_system_command in path traversal tests so paths that pass validation but don't exist return 404 deterministically. - Add test for firmware_bin_path=None case. --- esphome/dashboard/web_server.py | 4 ++++ tests/dashboard/test_web_server.py | 36 ++++++++++++++++++++++++++++-- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/esphome/dashboard/web_server.py b/esphome/dashboard/web_server.py index 24a0bacf05..a12b2fd579 100644 --- a/esphome/dashboard/web_server.py +++ b/esphome/dashboard/web_server.py @@ -1063,6 +1063,10 @@ class DownloadBinaryRequestHandler(BaseHandler): f"{storage_json.name}-{file_name}", ) + if storage_json.firmware_bin_path is None: + self.send_error(404) + return + base_dir = storage_json.firmware_bin_path.parent.resolve() path = base_dir.joinpath(file_name).resolve() try: diff --git a/tests/dashboard/test_web_server.py b/tests/dashboard/test_web_server.py index d216652162..19e19c7d8d 100644 --- a/tests/dashboard/test_web_server.py +++ b/tests/dashboard/test_web_server.py @@ -573,8 +573,16 @@ async def test_download_binary_handler_path_traversal_protection( mock_storage.firmware_bin_path = firmware_file mock_storage_json.load.return_value = mock_storage - # Attempt path traversal attack - should be blocked - with pytest.raises(HTTPClientError) as exc_info: + # Mock async_run_system_command so paths that pass validation but don't exist + # return 404 deterministically without spawning a real subprocess. + with ( + patch( + "esphome.dashboard.web_server.async_run_system_command", + new_callable=AsyncMock, + return_value=(2, "", ""), + ), + pytest.raises(HTTPClientError) as exc_info, + ): await dashboard.fetch( f"/download.bin?configuration=test.yaml&file={attack_path}", method="GET", @@ -582,6 +590,30 @@ async def test_download_binary_handler_path_traversal_protection( assert exc_info.value.code == expected_code +@pytest.mark.asyncio +@pytest.mark.usefixtures("mock_ext_storage_path") +async def test_download_binary_handler_no_firmware_bin_path( + dashboard: DashboardTestHelper, + mock_storage_json: MagicMock, +) -> None: + """Test that download returns 404 when firmware_bin_path is None. + + This covers configs created by StorageJSON.from_wizard() where no + firmware has been compiled yet. + """ + mock_storage = Mock() + mock_storage.name = "test_device" + mock_storage.firmware_bin_path = None + mock_storage_json.load.return_value = mock_storage + + with pytest.raises(HTTPClientError) as exc_info: + await dashboard.fetch( + "/download.bin?configuration=test.yaml&file=firmware.bin", + method="GET", + ) + assert exc_info.value.code == 404 + + @pytest.mark.asyncio @pytest.mark.usefixtures("mock_ext_storage_path") async def test_download_binary_handler_multiple_subdirectory_levels( From bf7ede1d43c73d83aeed85dd9a107410059e3c63 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:24:11 -0600 Subject: [PATCH 19/30] Make mock_token_hex strict on unexpected nbytes Raise ValueError for unexpected nbytes values so tests fail clearly if production code starts calling token_hex with an incorrect size. --- tests/unit_tests/test_espota2.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/unit_tests/test_espota2.py b/tests/unit_tests/test_espota2.py index 57e33e083e..20ba4b1f76 100644 --- a/tests/unit_tests/test_espota2.py +++ b/tests/unit_tests/test_espota2.py @@ -61,7 +61,9 @@ def mock_token_hex() -> Generator[Mock]: def _token_hex(nbytes: int) -> str: if nbytes == 16: return MOCK_MD5_CNONCE - return MOCK_SHA256_CNONCE + if nbytes == 32: + return MOCK_SHA256_CNONCE + raise ValueError(f"Unexpected nbytes for token_hex mock: {nbytes}") with patch("esphome.espota2.secrets.token_hex", side_effect=_token_hex) as mock: yield mock From ea99593575b14e06503ce92d95a275e3421609e0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:24:44 -0600 Subject: [PATCH 20/30] Build auth_settings on dashboard_settings and monkeypatch env - Refactor auth_settings fixture to extend dashboard_settings instead of duplicating setup. - Explicitly clear DISABLE_HA_AUTHENTICATION in HA add-on test to prevent order-dependent flakiness. --- tests/dashboard/test_settings.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/tests/dashboard/test_settings.py b/tests/dashboard/test_settings.py index a7594e4222..3ca57716ec 100644 --- a/tests/dashboard/test_settings.py +++ b/tests/dashboard/test_settings.py @@ -225,16 +225,12 @@ def test_config_path_parent_resolves_to_config_dir(tmp_path: Path) -> None: @pytest.fixture -def auth_settings(tmp_path: Path) -> DashboardSettings: - """Create DashboardSettings with auth configured.""" - settings = DashboardSettings() - resolved_dir = tmp_path.resolve() - settings.config_dir = resolved_dir - settings.absolute_config_dir = resolved_dir - settings.username = "admin" - settings.using_password = True - settings.password_hash = password_hash("correctpassword") - return settings +def auth_settings(dashboard_settings: DashboardSettings) -> DashboardSettings: + """Create DashboardSettings with auth configured, based on dashboard_settings.""" + dashboard_settings.username = "admin" + dashboard_settings.using_password = True + dashboard_settings.password_hash = password_hash("correctpassword") + return dashboard_settings def test_check_password_correct_credentials(auth_settings: DashboardSettings) -> None: @@ -264,6 +260,7 @@ def test_check_password_no_auth(dashboard_settings: DashboardSettings) -> None: def test_check_password_ha_addon_no_password( dashboard_settings: DashboardSettings, + monkeypatch: pytest.MonkeyPatch, ) -> None: """Test check_password doesn't crash in HA add-on mode without a password. @@ -271,6 +268,7 @@ def test_check_password_ha_addon_no_password( is False, leaving password_hash as b"". This must not raise TypeError in hmac.compare_digest. """ + monkeypatch.delenv("DISABLE_HA_AUTHENTICATION", raising=False) dashboard_settings.on_ha_addon = True dashboard_settings.using_password = False # password_hash stays as default b"" From 4795971f1c6e40e2b51cc34936e0869893f78394 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:25:19 -0600 Subject: [PATCH 21/30] Use usefixtures for tests that don't reference mock_auth_settings Replace unused mock_auth_settings parameter with @pytest.mark.usefixtures decorator to avoid PLW0613 lint warnings. --- tests/dashboard/test_web_server.py | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/tests/dashboard/test_web_server.py b/tests/dashboard/test_web_server.py index 66497088fb..403176187f 100644 --- a/tests/dashboard/test_web_server.py +++ b/tests/dashboard/test_web_server.py @@ -1699,25 +1699,22 @@ def mock_auth_settings(mock_dashboard_settings: MagicMock) -> MagicMock: return mock_dashboard_settings -def test_is_authenticated_malformed_base64( - mock_auth_settings: MagicMock, -) -> None: +@pytest.mark.usefixtures("mock_auth_settings") +def test_is_authenticated_malformed_base64() -> None: """Test that invalid base64 in Authorization header returns False.""" handler = _make_auth_handler("Basic !!!not-valid-base64!!!") assert web_server.is_authenticated(handler) is False -def test_is_authenticated_bad_base64_padding( - mock_auth_settings: MagicMock, -) -> None: +@pytest.mark.usefixtures("mock_auth_settings") +def test_is_authenticated_bad_base64_padding() -> None: """Test that incorrect base64 padding (binascii.Error) returns False.""" handler = _make_auth_handler("Basic abc") assert web_server.is_authenticated(handler) is False -def test_is_authenticated_invalid_utf8( - mock_auth_settings: MagicMock, -) -> None: +@pytest.mark.usefixtures("mock_auth_settings") +def test_is_authenticated_invalid_utf8() -> None: """Test that base64 decoding to invalid UTF-8 returns False.""" # \xff\xfe is invalid UTF-8 bad_payload = base64.b64encode(b"\xff\xfe").decode("ascii") @@ -1725,9 +1722,8 @@ def test_is_authenticated_invalid_utf8( assert web_server.is_authenticated(handler) is False -def test_is_authenticated_no_colon( - mock_auth_settings: MagicMock, -) -> None: +@pytest.mark.usefixtures("mock_auth_settings") +def test_is_authenticated_no_colon() -> None: """Test that base64 payload without ':' separator returns False.""" no_colon = base64.b64encode(b"nocolonhere").decode("ascii") handler = _make_auth_handler(f"Basic {no_colon}") From bb2d7c97421622de6dff236e63ac1835735c8059 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:25:50 -0600 Subject: [PATCH 22/30] Use full-width accumulator and iterate max_len in constant-time compare - Change result accumulator from volatile uint8_t to volatile size_t to prevent truncation bypass (e.g. digest_len + 256 XOR). - Iterate over max(digest_len, provided_len) so trailing bytes in either string are also compared. --- .../web_server_idf/web_server_idf.cpp | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/esphome/components/web_server_idf/web_server_idf.cpp b/esphome/components/web_server_idf/web_server_idf.cpp index 7cb56def57..8350baa909 100644 --- a/esphome/components/web_server_idf/web_server_idf.cpp +++ b/esphome/components/web_server_idf/web_server_idf.cpp @@ -356,19 +356,21 @@ bool AsyncWebServerRequest::authenticate(const char *username, const char *passw // Constant-time comparison to avoid timing side channels. // No early return on length mismatch — the length difference is folded - // into the accumulator so the loop always runs over the full digest. + // into the accumulator and the loop runs over the longer of the two + // strings so extra trailing bytes are also compared. const char *provided = auth_str + auth_prefix_len; size_t digest_len = out; // length from esp_crypto_base64_encode size_t provided_len = strlen(provided); - volatile uint8_t result = 0; - // Non-zero if lengths differ; XOR of two size_t values truncated to 8 bits - // catches differences in the low byte, and any length mismatch also causes - // the byte-wise loop below to accumulate non-zero XOR values. - result |= static_cast(digest_len ^ provided_len); - for (size_t i = 0; i < digest_len; i++) { - // Bounds-safe: use 0 for bytes beyond provided_len + // Use full-width XOR so any bit difference in the lengths is preserved + // (uint8_t truncation would miss differences in higher bytes, e.g. + // digest_len vs digest_len + 256). + volatile size_t result = digest_len ^ provided_len; + size_t max_len = (digest_len > provided_len) ? digest_len : provided_len; + for (size_t i = 0; i < max_len; i++) { + // Bounds-safe: substitute 0 for bytes beyond each string's length + char digest_ch = (i < digest_len) ? digest.get()[i] : 0; char provided_ch = (i < provided_len) ? provided[i] : 0; - result |= static_cast(digest.get()[i] ^ provided_ch); + result |= static_cast(digest_ch ^ provided_ch); } return result == 0; } From b650d2df31c534911f8016fef8ba4b6fa832b481 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:32:00 -0600 Subject: [PATCH 23/30] Reject empty file names and fix FlashImage.path endswith call - Return 400 for empty or whitespace-only file_name to prevent the idedata fallback from matching everything via empty-string suffix. - Use image.path.as_posix().endswith() since FlashImage.path is a Path object which does not have a string endswith method. - Add parametrized test for empty/whitespace file name values. --- esphome/dashboard/web_server.py | 4 ++-- tests/dashboard/test_web_server.py | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/esphome/dashboard/web_server.py b/esphome/dashboard/web_server.py index a12b2fd579..00974bf460 100644 --- a/esphome/dashboard/web_server.py +++ b/esphome/dashboard/web_server.py @@ -1054,7 +1054,7 @@ class DownloadBinaryRequestHandler(BaseHandler): # fallback to type=, but prioritize file= file_name = self.get_argument("type", None) file_name = self.get_argument("file", file_name) - if file_name is None: + if file_name is None or not file_name.strip(): self.send_error(400) return # get requested download name, or build it based on filename @@ -1087,7 +1087,7 @@ class DownloadBinaryRequestHandler(BaseHandler): found = False for image in idedata.extra_flash_images: - if image.path.endswith(file_name): + if image.path.as_posix().endswith(file_name): path = image.path download_name = file_name found = True diff --git a/tests/dashboard/test_web_server.py b/tests/dashboard/test_web_server.py index 19e19c7d8d..95afe19899 100644 --- a/tests/dashboard/test_web_server.py +++ b/tests/dashboard/test_web_server.py @@ -614,6 +614,28 @@ async def test_download_binary_handler_no_firmware_bin_path( assert exc_info.value.code == 404 +@pytest.mark.asyncio +@pytest.mark.usefixtures("mock_ext_storage_path") +@pytest.mark.parametrize("file_value", ["", " ", "%20"]) +async def test_download_binary_handler_empty_file_name( + dashboard: DashboardTestHelper, + mock_storage_json: MagicMock, + file_value: str, +) -> None: + """Test that download returns 400 for empty or whitespace-only file names.""" + mock_storage = Mock() + mock_storage.name = "test_device" + mock_storage.firmware_bin_path = Path("/fake/firmware.bin") + mock_storage_json.load.return_value = mock_storage + + with pytest.raises(HTTPClientError) as exc_info: + await dashboard.fetch( + f"/download.bin?configuration=test.yaml&file={file_value}", + method="GET", + ) + assert exc_info.value.code == 400 + + @pytest.mark.asyncio @pytest.mark.usefixtures("mock_ext_storage_path") async def test_download_binary_handler_multiple_subdirectory_levels( From 30662bc11b284e1a5b89ddfef1aa9e01afaa5bca Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:34:14 -0600 Subject: [PATCH 24/30] Update module docstring to reflect auth test coverage --- tests/dashboard/test_settings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/dashboard/test_settings.py b/tests/dashboard/test_settings.py index 3ca57716ec..89f1b9a424 100644 --- a/tests/dashboard/test_settings.py +++ b/tests/dashboard/test_settings.py @@ -1,4 +1,4 @@ -"""Tests for dashboard settings Path-related functionality.""" +"""Tests for DashboardSettings (path resolution and authentication).""" from __future__ import annotations From 401d3c2056acf5c33eac4daceb71f133748f1467 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:36:02 -0600 Subject: [PATCH 25/30] Fix idedata test mock to use Path instead of str The test set mock_image.path to str, but FlashImage.path is a Path. This masked a pre-existing bug where Path.endswith() doesn't exist. Fix the mock to match the real type so as_posix() works correctly. --- tests/dashboard/test_web_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/dashboard/test_web_server.py b/tests/dashboard/test_web_server.py index 95afe19899..d21873cf61 100644 --- a/tests/dashboard/test_web_server.py +++ b/tests/dashboard/test_web_server.py @@ -422,7 +422,7 @@ async def test_download_binary_handler_idedata_fallback( # Mock idedata response mock_image = Mock() - mock_image.path = str(bootloader_file) + mock_image.path = bootloader_file mock_idedata_instance = Mock() mock_idedata_instance.extra_flash_images = [mock_image] mock_idedata.return_value = mock_idedata_instance From f2e1c2c6509a6a68799c340a82f0ad839694cc2a Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:39:09 -0600 Subject: [PATCH 26/30] Derive provided_len from string size and bound loop to digest_len - Use auth.value().size() instead of strlen() to avoid rescanning attacker-controlled header content. - Iterate over digest_len (expected length) instead of max_len so a long Authorization header cannot force extra work. The full-width length XOR already rejects any length mismatch. --- .../web_server_idf/web_server_idf.cpp | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/esphome/components/web_server_idf/web_server_idf.cpp b/esphome/components/web_server_idf/web_server_idf.cpp index 8350baa909..3e308fed79 100644 --- a/esphome/components/web_server_idf/web_server_idf.cpp +++ b/esphome/components/web_server_idf/web_server_idf.cpp @@ -356,21 +356,22 @@ bool AsyncWebServerRequest::authenticate(const char *username, const char *passw // Constant-time comparison to avoid timing side channels. // No early return on length mismatch — the length difference is folded - // into the accumulator and the loop runs over the longer of the two - // strings so extra trailing bytes are also compared. + // into the accumulator so any mismatch is rejected. const char *provided = auth_str + auth_prefix_len; size_t digest_len = out; // length from esp_crypto_base64_encode - size_t provided_len = strlen(provided); + // Derive provided_len from the already-sized std::string rather than + // rescanning with strlen (avoids attacker-controlled scan length). + size_t provided_len = auth.value().size() - auth_prefix_len; // Use full-width XOR so any bit difference in the lengths is preserved // (uint8_t truncation would miss differences in higher bytes, e.g. // digest_len vs digest_len + 256). volatile size_t result = digest_len ^ provided_len; - size_t max_len = (digest_len > provided_len) ? digest_len : provided_len; - for (size_t i = 0; i < max_len; i++) { - // Bounds-safe: substitute 0 for bytes beyond each string's length - char digest_ch = (i < digest_len) ? digest.get()[i] : 0; + // Iterate over the expected digest length only — the full-width length + // XOR above already rejects any length mismatch, and bounding the loop + // prevents a long Authorization header from forcing extra work. + for (size_t i = 0; i < digest_len; i++) { char provided_ch = (i < provided_len) ? provided[i] : 0; - result |= static_cast(digest_ch ^ provided_ch); + result |= static_cast(digest.get()[i] ^ provided_ch); } return result == 0; } From 5c5bf50e49a22360850453fc7cec21cff318c9e4 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:41:00 -0600 Subject: [PATCH 27/30] Update test docstring to reflect validation instead of sanitization --- tests/dashboard/test_web_server.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/dashboard/test_web_server.py b/tests/dashboard/test_web_server.py index d21873cf61..2924e09f77 100644 --- a/tests/dashboard/test_web_server.py +++ b/tests/dashboard/test_web_server.py @@ -552,8 +552,8 @@ async def test_download_binary_handler_path_traversal_protection( ) -> None: """Test that DownloadBinaryRequestHandler prevents path traversal attacks. - Verifies that attempts to use '..' in file paths are sanitized to prevent - accessing files outside the build directory. Tests multiple attack vectors. + Verifies that attempts to escape the build directory via '..' are rejected + using resolve()/relative_to() validation. Tests multiple attack vectors. Real traversals that escape the base directory get 403. Paths like '....' that resolve inside the base directory but don't exist get 404. """ From b8cad678b176517067dc5646adcd02d9fc7f53d0 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:48:41 -0600 Subject: [PATCH 28/30] URL-encode whitespace in empty file name test parameter Replace raw spaces with %20%20 to avoid flakiness from HTTP clients handling unencoded spaces differently. --- tests/dashboard/test_web_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/dashboard/test_web_server.py b/tests/dashboard/test_web_server.py index 2924e09f77..274cda3636 100644 --- a/tests/dashboard/test_web_server.py +++ b/tests/dashboard/test_web_server.py @@ -616,7 +616,7 @@ async def test_download_binary_handler_no_firmware_bin_path( @pytest.mark.asyncio @pytest.mark.usefixtures("mock_ext_storage_path") -@pytest.mark.parametrize("file_value", ["", " ", "%20"]) +@pytest.mark.parametrize("file_value", ["", "%20%20", "%20"]) async def test_download_binary_handler_empty_file_name( dashboard: DashboardTestHelper, mock_storage_json: MagicMock, From 4cdd73904f6022365d28f63db391aff8162868c3 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:49:53 -0600 Subject: [PATCH 29/30] Encode usernames as UTF-8 bytes for hmac.compare_digest hmac.compare_digest() on str inputs raises TypeError if either contains non-ASCII characters. Encode both sides as UTF-8 bytes. Add test with non-ASCII username to prevent regressions. --- esphome/dashboard/settings.py | 4 +++- tests/dashboard/test_settings.py | 12 ++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/esphome/dashboard/settings.py b/esphome/dashboard/settings.py index aa994ae487..3b22180b1d 100644 --- a/esphome/dashboard/settings.py +++ b/esphome/dashboard/settings.py @@ -85,7 +85,9 @@ class DashboardSettings: if not self.using_auth: return True # Compare in constant running time (to prevent timing attacks) - username_matches = hmac.compare_digest(username, self.username) + username_matches = hmac.compare_digest( + username.encode("utf-8"), self.username.encode("utf-8") + ) password_matches = hmac.compare_digest( self.password_hash, password_hash(password) ) diff --git a/tests/dashboard/test_settings.py b/tests/dashboard/test_settings.py index 89f1b9a424..55776ac7c4 100644 --- a/tests/dashboard/test_settings.py +++ b/tests/dashboard/test_settings.py @@ -258,6 +258,18 @@ def test_check_password_no_auth(dashboard_settings: DashboardSettings) -> None: assert dashboard_settings.check_password("anyone", "anything") is True +def test_check_password_non_ascii_username( + dashboard_settings: DashboardSettings, +) -> None: + """Test check_password handles non-ASCII usernames without TypeError.""" + dashboard_settings.username = "\u00e9l\u00e8ve" + dashboard_settings.using_password = True + dashboard_settings.password_hash = password_hash("pass") + assert dashboard_settings.check_password("\u00e9l\u00e8ve", "pass") is True + assert dashboard_settings.check_password("\u00e9l\u00e8ve", "wrong") is False + assert dashboard_settings.check_password("other", "pass") is False + + def test_check_password_ha_addon_no_password( dashboard_settings: DashboardSettings, monkeypatch: pytest.MonkeyPatch, From 2ceb6ee95b1abfac7e323906f6df2b4d7fdec529 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 8 Feb 2026 07:55:48 -0600 Subject: [PATCH 30/30] Add comment explaining Windows-specific multiple_dots behavior On Windows, Path.resolve() treats '....' as parent traversal (403), while on Unix it is a literal directory name that stays inside the base directory (404). --- tests/dashboard/test_web_server.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/dashboard/test_web_server.py b/tests/dashboard/test_web_server.py index 274cda3636..9ea7a5164b 100644 --- a/tests/dashboard/test_web_server.py +++ b/tests/dashboard/test_web_server.py @@ -538,6 +538,10 @@ async def test_download_binary_handler_subdirectory_file_url_encoded( pytest.param("//etc/passwd", 403, id="double_slash_absolute"), pytest.param( "....//secrets.yaml", + # On Windows, Path.resolve() treats "..." and "...." as parent + # traversal (like ".."), so the path escapes base_dir -> 403. + # On Unix, "...." is a literal directory name that stays inside + # base_dir but doesn't exist -> 404. 403 if sys.platform == "win32" else 404, id="multiple_dots", ),