1
0
mirror of https://github.com/esphome/esphome.git synced 2026-02-09 09:11:52 +00:00

Compare commits

...

9 Commits

Author SHA1 Message Date
J. Nick Koston
2ceb6ee95b 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).
2026-02-08 07:55:48 -06:00
J. Nick Koston
b8cad678b1 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.
2026-02-08 07:48:41 -06:00
J. Nick Koston
5c5bf50e49 Update test docstring to reflect validation instead of sanitization 2026-02-08 07:41:00 -06:00
J. Nick Koston
401d3c2056 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.
2026-02-08 07:36:02 -06:00
J. Nick Koston
b650d2df31 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.
2026-02-08 07:32:00 -06:00
J. Nick Koston
43448d55f1 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.
2026-02-08 07:23:24 -06:00
J. Nick Koston
e362e6fe2f 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.
2026-02-08 07:22:08 -06:00
J. Nick Koston
803b9a7a18 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.
2026-02-08 07:01:37 -06:00
J. Nick Koston
a40c87eeed [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.
2026-02-08 06:48:38 -06:00
2 changed files with 92 additions and 18 deletions

View File

@@ -1054,17 +1054,26 @@ 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
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)
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:
path.relative_to(base_dir)
except ValueError:
self.send_error(403)
return
if not path.is_file():
args = ["esphome", "idedata", settings.rel_path(configuration)]
@@ -1078,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

View File

@@ -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
@@ -421,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
@@ -528,14 +529,22 @@ 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",
# 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",
),
],
)
async def test_download_binary_handler_path_traversal_protection(
@@ -543,11 +552,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.
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.
"""
# Create build structure
build_dir = get_build_path(tmp_path, "test")
@@ -565,14 +577,67 @@ 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",
)
# 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
@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")
@pytest.mark.parametrize("file_value", ["", "%20%20", "%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