Compare commits

...

5 Commits

Author SHA1 Message Date
Varun Chawla
2a65905998
Merge 2657155dfc into b08debceca 2026-07-06 17:34:07 +08:00
Varun Chawla
2657155dfc
fix: block Windows drive-qualified paths in path traversal validation
PurePosixPath doesn't understand Windows drive letters, so paths like
C:/Windows/secret.png or C:secret.png would pass validation but escape
the output directory when joined via os.path.join on Windows.

Add PureWindowsPath check to detect drive-qualified paths on all
platforms. Added tests for Windows drive absolute, relative, and
backslash path variants.
2026-03-24 21:39:24 -07:00
Varun Chawla
d29f0228a8
refactor: use PurePosixPath for path traversal validation per review feedback 2026-03-17 22:19:08 -07:00
Varun Chawla
95c511e167
security: handle Windows backslash path traversal in filename validation
Normalize backslashes to forward slashes before checking for path
traversal patterns, preventing attacks like `folder\..\secret` that
bypass forward-slash-only checks on Windows. Addresses review feedback
from light-and-ray on PR #12353.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-17 22:18:30 -07:00
Varun Chawla
64e1c0a0a6
security: refine path traversal validation to allow consecutive dots in filenames
Fixes #12352

The previous validation incorrectly rejected filenames with consecutive dots
(e.g., test..png) by checking if '..' exists anywhere in the filename.

This commit refines the validation to:
- Block actual path traversal patterns: '../', '/..'
- Block filenames starting with '..' (e.g., '..secret')
- Block absolute paths starting with '/'
- Allow consecutive dots in filenames (e.g., 'test..png', 'my...file.jpg')

Changes:
- Updated validation logic in /view and /upload/mask endpoints
- Added comprehensive test suite covering both security and functionality
- All tests pass: blocks path traversal, allows valid filenames with dots
2026-03-17 22:18:02 -07:00
2 changed files with 177 additions and 2 deletions

View File

@ -2,6 +2,7 @@ import errno
import os
import sys
import asyncio
from pathlib import PurePosixPath, PureWindowsPath
import traceback
import time
@ -475,7 +476,11 @@ class PromptServer():
return web.Response(status=400)
# validation for security: prevent accessing arbitrary path
if filename[0] == '/' or '..' in filename:
# Normalize backslashes and use standard library to parse path components
normalized = filename.replace('\\', '/')
path = PurePosixPath(normalized)
win_path = PureWindowsPath(normalized)
if path.is_absolute() or win_path.is_absolute() or win_path.drive or '..' in path.parts:
return web.Response(status=400)
if output_dir is None:
@ -532,7 +537,10 @@ class PromptServer():
return web.Response(status=400)
# validation for security: prevent accessing arbitrary path
if filename[0] == '/' or '..' in filename:
# Normalize backslashes and use standard library to parse path components
normalized = filename.replace('\\', '/')
path = PurePosixPath(normalized)
if path.is_absolute() or '..' in path.parts:
return web.Response(status=400)
if output_dir is None:

View File

@ -0,0 +1,167 @@
"""Tests for /view endpoint path traversal validation"""
import pytest
from aiohttp import web
from aiohttp.test_utils import AioHTTPTestCase, unittest_run_loop
import os
import tempfile
from pathlib import Path, PurePosixPath, PureWindowsPath
class TestViewEndpointSecurity(AioHTTPTestCase):
"""Test /view endpoint path traversal validation"""
async def get_application(self):
"""Create a minimal test application with /view endpoint"""
app = web.Application()
# Create a test directory with test files
self.test_dir = tempfile.mkdtemp()
self.test_file_valid = os.path.join(self.test_dir, "test..png")
self.test_file_normal = os.path.join(self.test_dir, "normal.png")
self.test_file_dotsecret = os.path.join(self.test_dir, "..secret")
# Create test files
Path(self.test_file_valid).touch()
Path(self.test_file_normal).touch()
Path(self.test_file_dotsecret).touch()
async def view_handler(request):
"""Simplified /view endpoint handler for testing"""
if "filename" not in request.rel_url.query:
return web.Response(status=404)
filename = request.rel_url.query["filename"]
if not filename:
return web.Response(status=400)
# validation for security: prevent accessing arbitrary path
# Normalize backslashes and use standard library to parse path components
normalized = filename.replace('\\', '/')
path = PurePosixPath(normalized)
win_path = PureWindowsPath(normalized)
if path.is_absolute() or win_path.is_absolute() or win_path.drive or '..' in path.parts:
return web.Response(status=400)
# For testing, just check if file exists in test directory
filename = os.path.basename(filename)
file_path = os.path.join(self.test_dir, filename)
if os.path.isfile(file_path):
return web.Response(status=200, text="OK")
return web.Response(status=404)
app.router.add_get('/view', view_handler)
return app
async def tearDownAsync(self):
"""Clean up test files"""
import shutil
if hasattr(self, 'test_dir') and os.path.exists(self.test_dir):
shutil.rmtree(self.test_dir)
@unittest_run_loop
async def test_allows_consecutive_dots_in_filename(self):
"""Test that files with consecutive dots (e.g., test..png) are allowed"""
resp = await self.client.request("GET", "/view?filename=test..png")
assert resp.status == 200, "Should allow files with consecutive dots in filename"
@unittest_run_loop
async def test_allows_normal_filenames(self):
"""Test that normal filenames work"""
resp = await self.client.request("GET", "/view?filename=normal.png")
assert resp.status == 200, "Should allow normal filenames"
@unittest_run_loop
async def test_blocks_path_traversal_dotdot_slash(self):
"""Test that path traversal with ../ is blocked"""
resp = await self.client.request("GET", "/view?filename=../etc/passwd")
assert resp.status == 400, "Should block ../ path traversal"
@unittest_run_loop
async def test_blocks_path_traversal_slash_dotdot(self):
"""Test that path traversal with /.. is blocked"""
resp = await self.client.request("GET", "/view?filename=folder/../etc/passwd")
assert resp.status == 400, "Should block /.. path traversal"
@unittest_run_loop
async def test_blocks_absolute_paths(self):
"""Test that absolute paths starting with / are blocked"""
resp = await self.client.request("GET", "/view?filename=/etc/passwd")
assert resp.status == 400, "Should block absolute paths"
@unittest_run_loop
async def test_allows_dotdot_prefix_in_filename(self):
"""Test that filenames starting with .. but not as a path component are allowed"""
resp = await self.client.request("GET", "/view?filename=..secret")
assert resp.status == 200, "Should allow filenames starting with .. that aren't path traversal"
@unittest_run_loop
async def test_blocks_bare_dotdot(self):
"""Test that bare .. as a path component is blocked"""
resp = await self.client.request("GET", "/view?filename=..")
assert resp.status == 400, "Should block bare .. path component"
@unittest_run_loop
async def test_multiple_consecutive_dots(self):
"""Test that multiple consecutive dots in filename are allowed"""
# Create a file with multiple dots
test_file = os.path.join(self.test_dir, "test...png")
Path(test_file).touch()
resp = await self.client.request("GET", "/view?filename=test...png")
assert resp.status == 200, "Should allow files with multiple consecutive dots"
@unittest_run_loop
async def test_dots_in_middle_of_filename(self):
"""Test that dots in the middle of filename are allowed"""
# Create a file with dots in middle
test_file = os.path.join(self.test_dir, "my..file..name.png")
Path(test_file).touch()
resp = await self.client.request("GET", "/view?filename=my..file..name.png")
assert resp.status == 200, "Should allow dots in middle of filename"
@unittest_run_loop
async def test_blocks_backslash_path_traversal(self):
"""Test that Windows-style backslash path traversal (\\..) is blocked"""
resp = await self.client.request("GET", "/view?filename=folder%5C..%5Csecret")
assert resp.status == 400, "Should block backslash path traversal"
@unittest_run_loop
async def test_blocks_backslash_dotdot_at_start(self):
"""Test that backslash path traversal starting with ..\\ is blocked"""
resp = await self.client.request("GET", "/view?filename=..%5Cetc%5Cpasswd")
assert resp.status == 400, "Should block ..\\ path traversal at start"
@unittest_run_loop
async def test_blocks_mixed_slash_backslash_traversal(self):
"""Test that mixed forward/backslash path traversal is blocked"""
resp = await self.client.request("GET", "/view?filename=folder/..%5Csecret")
assert resp.status == 400, "Should block mixed slash/backslash path traversal"
@unittest_run_loop
async def test_blocks_backslash_absolute_path(self):
"""Test that Windows absolute paths with backslash are blocked (e.g., C:\\)"""
resp = await self.client.request("GET", "/view?filename=%5Cetc%5Cpasswd")
assert resp.status == 400, "Should block backslash absolute paths"
@unittest_run_loop
async def test_blocks_windows_drive_absolute_path(self):
"""Test that Windows drive-qualified absolute paths (C:/...) are blocked"""
resp = await self.client.request("GET", "/view?filename=C:/Windows/secret.png")
assert resp.status == 400, "Should block Windows drive-qualified absolute paths"
@unittest_run_loop
async def test_blocks_windows_drive_relative_path(self):
"""Test that Windows drive-qualified relative paths (C:secret.png) are blocked"""
resp = await self.client.request("GET", "/view?filename=C:secret.png")
assert resp.status == 400, "Should block Windows drive-qualified relative paths"
@unittest_run_loop
async def test_blocks_windows_drive_backslash_path(self):
"""Test that Windows drive paths with backslashes are blocked"""
resp = await self.client.request("GET", "/view?filename=C:%5CWindows%5Csecret.png")
assert resp.status == 400, "Should block Windows drive backslash paths"