Skip to content

Commit be3f8c0

Browse files
committed
Security fix: prevent path traversal and extension bypass
- Sanitize and extracted folder via secure_filename() - Re-validate extensions after name override - Enforce upload path containment - Add security regression tests - Credit: Jaron Cabral (Cal Poly Humboldt) for discovery and reporting
1 parent 9e813e9 commit be3f8c0

File tree

3 files changed

+302
-5
lines changed

3 files changed

+302
-5
lines changed

CHANGES.rst

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,25 @@ Changelog
55
------------------
66
- drop support for Python 3.8 and 3.9
77
- add support for Python 3.13
8-
9-
1.4.1 (unreleased)
10-
------------------
118
- migrate from setup.py to pyproject.toml configuration
129
- fix doc building on read the docs
10+
- **SECURITY FIX**: Fix critical path traversal and extension bypass vulnerability (CVE pending, CVSS 9.8)
11+
12+
- Apply ``secure_filename()`` to the ``name`` parameter to prevent path traversal attacks
13+
- Re-validate file extension after ``name`` override to prevent extension bypass
14+
- Add path containment check to ensure files are saved within the upload directory
15+
- Sanitize folder component when extracted from ``name`` parameter
16+
17+
**Impact**: This vulnerability allowed remote attackers to write files to arbitrary locations
18+
on the filesystem and bypass extension restrictions, potentially leading to remote code
19+
execution via Server-Side Template Injection (SSTI) in Flask applications.
20+
21+
**Credit**: Jaron Cabral (Cal Poly Humboldt) for discovery and reporting
22+
23+
**Recommendation**: All users should upgrade to this version immediately. Do not pass
24+
user-controlled input to the ``name`` parameter in older versions.
25+
26+
1327

1428
1.4.0 (2023.10.03)
1529
------------------

src/flask_uploads/flask_uploads.py

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,8 +309,18 @@ def save(
309309
if not isinstance(storage, FileStorage):
310310
raise TypeError("storage must be a werkzeug.FileStorage")
311311

312+
# Track if name ends with dot before any processing
313+
name_ends_with_dot = name is not None and name.rstrip().endswith('.')
314+
312315
if folder is None and name is not None and "/" in name:
313316
folder, name = os.path.split(name)
317+
# Check again after split
318+
name_ends_with_dot = name.rstrip().endswith('.')
319+
# Sanitize folder and name extracted from name parameter
320+
if folder:
321+
folder = secure_filename(folder)
322+
if name:
323+
name = secure_filename(name)
314324
if storage.filename is None:
315325
raise ValueError("Filename must not be empty!")
316326
basename = self.get_basename(storage.filename)
@@ -319,12 +329,27 @@ def save(
319329
raise UploadNotAllowed()
320330

321331
if name:
322-
if name.endswith('.'):
323-
basename = name + extension(basename)
332+
# Sanitize name parameter to prevent path traversal
333+
name = secure_filename(name)
334+
if not name:
335+
raise ValueError("Invalid filename after sanitization")
336+
337+
if name_ends_with_dot and not name.endswith('.'):
338+
# Restore the dot if it was removed by secure_filename
339+
basename = name + '.' + extension(basename)
324340
else:
325341
basename = name
326342

343+
# Re-validate extension after name override
344+
ext = extension(basename)
345+
if ext and not self.extension_allowed(ext):
346+
raise UploadNotAllowed(
347+
f"File extension '{ext}' is not allowed"
348+
)
349+
327350
if folder:
351+
# Additional sanitization of folder parameter
352+
folder = secure_filename(folder)
328353
target_folder = os.path.join(self.config.destination, folder)
329354
else:
330355
target_folder = self.config.destination
@@ -334,6 +359,18 @@ def save(
334359
basename = self.resolve_conflict(target_folder, basename)
335360

336361
target = os.path.join(target_folder, basename)
362+
363+
# Verify path containment to prevent directory traversal
364+
target_real = os.path.realpath(target)
365+
dest_real = os.path.realpath(self.config.destination)
366+
if not (
367+
target_real.startswith(dest_real + os.sep) or
368+
target_real == dest_real
369+
):
370+
raise ValueError(
371+
"Security: Path traversal attempt detected"
372+
)
373+
337374
storage.save(target)
338375
if folder:
339376
return posixpath.join(folder, basename)

tests/test_flask_reuploaded.py

Lines changed: 246 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,16 @@
66

77
import os
88
import os.path
9+
import shutil
10+
import tempfile
911
from unittest.mock import Mock
1012
from unittest.mock import patch
1113

1214
import pytest
1315
from flask import Flask
1416
from flask import url_for
1517
from flask_uploads import ALL
18+
from flask_uploads import IMAGES
1619
from flask_uploads import AllExcept
1720
from flask_uploads import TestingFileStorage
1821
from flask_uploads import UploadConfiguration
@@ -394,3 +397,246 @@ def test_configure_for_set_throws_runtimeerror() -> None:
394397
app = Flask(__name__)
395398
with pytest.raises(RuntimeError):
396399
config_for_set(upload_set, app)
400+
401+
402+
class TestSecurityFixes:
403+
"""Tests for security vulnerability fixes.
404+
405+
These tests verify that path traversal and extension bypass vulnerabilities
406+
have been properly fixed.
407+
"""
408+
409+
def test_path_traversal_prevention_via_name_parameter(self) -> None:
410+
"""Verify path traversal via `name` is prevented."""
411+
with tempfile.TemporaryDirectory() as tmpdir:
412+
uset = UploadSet("files", ALL)
413+
uset._config = Config(tmpdir)
414+
tfs = TestingFileStorage(filename="safe.txt")
415+
416+
result = uset.save(tfs, name="../../../etc/passwd")
417+
418+
assert "../" not in result
419+
assert "passwd" in result
420+
assert tfs.saved is not None
421+
assert "passwd" in tfs.saved
422+
assert os.path.realpath(tfs.saved).startswith(
423+
os.path.realpath(tmpdir)
424+
)
425+
426+
def test_absolute_path_prevention_via_name_parameter(self) -> None:
427+
"""Verify absolute paths in `name` are sanitized."""
428+
with tempfile.TemporaryDirectory() as tmpdir:
429+
uset = UploadSet("files", ALL)
430+
uset._config = Config(tmpdir)
431+
tfs = TestingFileStorage(filename="safe.txt")
432+
433+
result = uset.save(tfs, name="/etc/passwd")
434+
435+
assert "passwd" in result
436+
assert tfs.saved is not None
437+
assert "passwd" in tfs.saved
438+
439+
def test_extension_bypass_prevention_via_name_parameter(self) -> None:
440+
"""Verify extension validation cannot be bypassed via `name`."""
441+
with tempfile.TemporaryDirectory() as tmpdir:
442+
uset = UploadSet("photos", IMAGES)
443+
uset._config = Config(tmpdir)
444+
tfs = TestingFileStorage(filename="legitimate.jpg")
445+
446+
with pytest.raises(UploadNotAllowed) as exc_info:
447+
uset.save(tfs, name="backdoor.py")
448+
449+
assert "py" in str(exc_info.value).lower()
450+
451+
def test_extension_bypass_with_double_extension(self) -> None:
452+
"""Verify double extensions don't bypass validation."""
453+
with tempfile.TemporaryDirectory() as tmpdir:
454+
uset = UploadSet("photos", IMAGES)
455+
uset._config = Config(tmpdir)
456+
tfs = TestingFileStorage(filename="safe.jpg")
457+
458+
result = uset.save(tfs, name="backdoor.php.jpg")
459+
assert ".jpg" in result
460+
461+
def test_folder_extraction_sanitization(self) -> None:
462+
"""Verify folder extracted from `name` is sanitized."""
463+
with tempfile.TemporaryDirectory() as tmpdir:
464+
uset = UploadSet("files", ALL)
465+
uset._config = Config(tmpdir)
466+
tfs = TestingFileStorage(filename="test.txt")
467+
468+
result = uset.save(tfs, name="../../tmp/file.txt")
469+
470+
assert "../" not in result
471+
assert tfs.saved is not None
472+
assert os.path.realpath(tfs.saved).startswith(
473+
os.path.realpath(tmpdir)
474+
)
475+
assert "file.txt" in result
476+
477+
def test_explicit_folder_parameter_sanitization(self) -> None:
478+
"""Verify explicit `folder` parameter is sanitized."""
479+
with tempfile.TemporaryDirectory() as tmpdir:
480+
uset = UploadSet("files", ALL)
481+
uset._config = Config(tmpdir)
482+
tfs = TestingFileStorage(filename="test.txt")
483+
484+
result = uset.save(tfs, folder="../../tmp")
485+
486+
assert "../" not in result
487+
assert tfs.saved is not None
488+
assert os.path.realpath(tfs.saved).startswith(
489+
os.path.realpath(tmpdir)
490+
)
491+
492+
def test_path_containment_check(self) -> None:
493+
"""Verify final path is contained within upload directory."""
494+
with tempfile.TemporaryDirectory() as tmpdir:
495+
uset = UploadSet("files", ALL)
496+
uset._config = Config(tmpdir)
497+
tfs = TestingFileStorage(filename="test.txt")
498+
499+
uset.save(tfs, name="../../../../../../../../tmp/escape.txt")
500+
501+
assert tfs.saved is not None
502+
real_saved = os.path.realpath(tfs.saved)
503+
real_upload = os.path.realpath(tmpdir)
504+
assert real_saved.startswith(real_upload)
505+
506+
def test_empty_name_after_sanitization(self) -> None:
507+
"""Verify names that become empty after sanitization are rejected."""
508+
with tempfile.TemporaryDirectory() as tmpdir:
509+
uset = UploadSet("files", ALL)
510+
uset._config = Config(tmpdir)
511+
tfs = TestingFileStorage(filename="test.txt")
512+
513+
with pytest.raises(ValueError) as exc_info:
514+
uset.save(tfs, name="...")
515+
516+
assert "sanitization" in str(exc_info.value).lower()
517+
518+
def test_windows_path_separators(self) -> None:
519+
"""Verify Windows-style path separators are sanitized."""
520+
with tempfile.TemporaryDirectory() as tmpdir:
521+
uset = UploadSet("files", ALL)
522+
uset._config = Config(tmpdir)
523+
tfs = TestingFileStorage(filename="test.txt")
524+
525+
result = uset.save(tfs, name="..\\..\\temp\\evil.txt")
526+
527+
assert "\\" not in result
528+
assert tfs.saved is not None
529+
assert os.path.realpath(tfs.saved).startswith(
530+
os.path.realpath(tmpdir)
531+
)
532+
533+
def test_legitimate_subfolder_still_works(self) -> None:
534+
"""Verify legitimate subfolder usage still works."""
535+
with tempfile.TemporaryDirectory() as tmpdir:
536+
uset = UploadSet("files", ALL)
537+
uset._config = Config(tmpdir)
538+
tfs = TestingFileStorage(filename="photo.jpg")
539+
540+
result = uset.save(tfs, name="users/avatar.jpg")
541+
542+
assert result == "users/avatar.jpg"
543+
assert tfs.saved is not None
544+
assert "users" in tfs.saved
545+
assert "avatar.jpg" in tfs.saved
546+
547+
def test_legitimate_custom_name_still_works(self) -> None:
548+
"""Verify legitimate custom names still work."""
549+
with tempfile.TemporaryDirectory() as tmpdir:
550+
uset = UploadSet("files", ALL)
551+
uset._config = Config(tmpdir)
552+
tfs = TestingFileStorage(filename="upload.txt")
553+
554+
result = uset.save(tfs, name="renamed_file.txt")
555+
556+
assert result == "renamed_file.txt"
557+
assert tfs.saved is not None
558+
assert "renamed_file.txt" in tfs.saved
559+
560+
def test_legitimate_name_with_extension_placeholder(self) -> None:
561+
"""Verify trailing dot preserves extension."""
562+
with tempfile.TemporaryDirectory() as tmpdir:
563+
uset = UploadSet("photos", IMAGES)
564+
uset._config = Config(tmpdir)
565+
tfs = TestingFileStorage(filename="photo.jpg")
566+
567+
result = uset.save(tfs, name="image_123.")
568+
569+
assert result == "image_123.jpg"
570+
assert tfs.saved is not None
571+
assert "image_123.jpg" in tfs.saved
572+
573+
def test_combined_attack_prevention(self) -> None:
574+
"""Verify combined path traversal + extension bypass is prevented."""
575+
with tempfile.TemporaryDirectory() as tmpdir:
576+
uset = UploadSet("photos", IMAGES)
577+
uset._config = Config(tmpdir)
578+
tfs = TestingFileStorage(filename="payload.jpg")
579+
580+
with pytest.raises(UploadNotAllowed):
581+
uset.save(tfs, name="../templates/rce.html")
582+
583+
def test_null_byte_injection(self) -> None:
584+
"""Verify null byte injection is sanitized."""
585+
with tempfile.TemporaryDirectory() as tmpdir:
586+
uset = UploadSet("files", ALL)
587+
uset._config = Config(tmpdir)
588+
tfs = TestingFileStorage(filename="test.txt")
589+
590+
result = uset.save(tfs, name="file.txt\x00.jpg")
591+
592+
assert "\x00" not in result
593+
assert tfs.saved is not None
594+
assert os.path.realpath(tfs.saved).startswith(
595+
os.path.realpath(tmpdir)
596+
)
597+
598+
def test_special_characters_sanitization(self) -> None:
599+
"""Verify special characters are sanitized."""
600+
with tempfile.TemporaryDirectory() as tmpdir:
601+
uset = UploadSet("files", ALL)
602+
uset._config = Config(tmpdir)
603+
tfs = TestingFileStorage(filename="test.txt")
604+
605+
result = uset.save(tfs, name='file<>:"|?*.txt')
606+
607+
for char in '<>:"|?*':
608+
assert char not in result
609+
assert tfs.saved is not None
610+
assert os.path.realpath(tfs.saved).startswith(
611+
os.path.realpath(tmpdir)
612+
)
613+
614+
def test_name_already_ends_with_dot(self) -> None:
615+
"""Verify trailing dot keeps extension."""
616+
with tempfile.TemporaryDirectory() as tmpdir:
617+
uset = UploadSet("files", ALL)
618+
uset._config = Config(tmpdir)
619+
tfs = TestingFileStorage(filename="photo.jpg")
620+
621+
result = uset.save(tfs, name="myfile.")
622+
623+
assert result == "myfile.jpg"
624+
assert tfs.saved is not None
625+
assert "myfile.jpg" in tfs.saved
626+
627+
def test_symlink_path_traversal_prevention(self) -> None:
628+
"""Verify symlinks cannot be used to escape upload directory."""
629+
with tempfile.TemporaryDirectory() as tmpdir:
630+
outside_dir = tempfile.mkdtemp()
631+
try:
632+
symlink_path = os.path.join(tmpdir, "link")
633+
os.symlink(outside_dir, symlink_path)
634+
635+
uset = UploadSet("files", ALL)
636+
uset._config = Config(tmpdir)
637+
tfs = TestingFileStorage(filename="test.txt")
638+
639+
with pytest.raises(ValueError, match="Path traversal"):
640+
uset.save(tfs, folder="link", name="../../escape.txt")
641+
finally:
642+
shutil.rmtree(outside_dir, ignore_errors=True)

0 commit comments

Comments
 (0)