Skip to content

Fixed 4 critical bugs in main.py#1

Open
ranujasanmir wants to merge 2 commits into
pixcapsoft:mainfrom
ranujasanmir:main
Open

Fixed 4 critical bugs in main.py#1
ranujasanmir wants to merge 2 commits into
pixcapsoft:mainfrom
ranujasanmir:main

Conversation

@ranujasanmir
Copy link
Copy Markdown

@ranujasanmir ranujasanmir commented May 10, 2026

Security Fix: Path Traversal, Symlink Traversal, XSS, and Header Injection in main.py

Summary

This PR fixes 4 security vulnerabilities in main.py's HTTP request handler, ranging from critical path traversal bugs to stored XSS and HTTP header injection. No new dependencies are introduced — all fixes use Python's standard library (html, os.path.realpath, urllib.parse.quote).


Vulnerabilities Fixed

🔴 Fix #1 — Path Traversal via startswith() Prefix Bypass

Severity: Critical

The bug:

# OLD — string prefix check, not a path boundary check
if not full_path.startswith(os.path.normpath(matched_root)):
    send_404(self)
    return

str.startswith() matches on raw characters, not on path segments. If a shared root is /data/share, then a resolved path of /data/shareother/secret.txt passes the check because the string literally starts with /data/share. An attacker on the network could potentially access files outside the intended shared directory if the filesystem layout has directories whose names are prefixes of a shared root name.

The fix:
The traversal check is replaced entirely by _safe_join(), a new dedicated helper that appends os.sep before comparing, making the boundary exact:

root_prefix = real_root + os.sep
if candidate != real_root and not candidate.startswith(root_prefix):
    return None  # escape attempt

🔴 Fix #2 — Symlink Traversal (normpath does not resolve symlinks)

Severity: Critical

The bug:

# OLD — normpath collapses '..' but ignores symlinks
full_path = os.path.normpath(full_path)

os.path.normpath() only collapses redundant separators and ../.. components lexically. It does not follow symbolic links on disk. A symlink placed inside a shared folder and pointing to /etc/passwd (Linux) or C:\Windows\System32 (Windows) would resolve to a path that still passed the old startswith check, allowing its target to be read and downloaded.

The fix:
_safe_join() calls os.path.realpath() which fully resolves all symlinks before any comparison is made:

real_root = os.path.realpath(root)
candidate = os.path.realpath(os.path.join(real_root, rel_path))

🔴 Fix #3 — Stored XSS via Unescaped Filenames in HTML

Severity: Critical

The bug:

# OLD — raw filename embedded directly into HTML
rows += f"<td>{icon} <a href='{href}'>{entry.name}</a></td>"

File and directory names were interpolated into the HTML response without any escaping. A file named <script>alert(document.cookie)</script>.txt inside a shared folder would execute JavaScript in every browser that navigates to that directory. This is a stored XSS vulnerability affecting every connected device.

The fix:
All user-controlled values — filenames, directory names, hrefs, and the page title — are now passed through html.escape() before being placed in HTML context:

safe_entry_name = html.escape(entry.name)
href = html.escape(raw_href)
safe_title = html.escape(title)

URL values in href attributes are additionally processed with urllib.parse.quote() to ensure they are valid and cannot break out of the attribute.


🟠 Fix #4 — HTTP Header Injection via Content-Disposition Filename

Severity: High

The bug:

# OLD — raw filename embedded inside double-quotes
handler.send_header("Content-Disposition", f'attachment; filename="{filename}"')

A filename containing ", \r, or \n characters could break out of the filename token and inject arbitrary HTTP response headers or corrupt the header block entirely. On some HTTP stacks this can lead to response splitting.

The fix:
A new helper _safe_filename_header() produces a dual-token Content-Disposition value per RFC 6266 / RFC 5987:

  • filename — ASCII-only fallback with dangerous characters stripped, safe for legacy browsers
  • filename* — full UTF-8 filename percent-encoded per RFC 5987, safe for modern browsers
def _safe_filename_header(filename: str) -> str:
    ascii_name = "".join(
        c for c in filename
        if 32 <= ord(c) < 127 and c not in ('"', "'", '\\', '/', '\r', '\n')
    ) or "download"
    encoded_name = urllib.parse.quote(filename, safe="ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-._~")
    return f'attachment; filename="{ascii_name}"; filename*=UTF-8\'\'{encoded_name}'

Changes at a Glance

File Change
main.py Added import html
main.py Added _safe_join(root, rel_path) — path traversal + symlink guard
main.py Added _safe_filename_header(filename) — RFC 5987 Content-Disposition
main.py build_index_html() — all filenames and hrefs now html.escape()'d
main.py build_dir_html() — all filenames, hrefs, and title now html.escape()'d
main.py html_shell() — title parameter now html.escape()'d
main.py serve_file() — uses _safe_filename_header() instead of raw f-string
main.py do_GET() — replaced inline traversal check with _safe_join() call

No changes to gui.py. No new dependencies.


Testing

To verify the fixes manually:

Path traversal (#1 & #2):

# With a shared folder at /tmp/shared, create:
mkdir /tmp/shared_evil
ln -s /etc/passwd /tmp/shared/symlink_test

# These should both return 404, not file contents:
curl http://localhost:8765/shared/../shared_evil/
curl http://localhost:8765/shared/symlink_test

XSS (#3):

# Create a file with an XSS name inside a shared folder:
touch '/tmp/shared/<script>alert(1)<\/script>.txt'

# Navigate to the directory in a browser — the filename should appear
# as literal text, not execute as a script.

Header injection (#4):

# File whose name contains a quote and newline:
# (create programmatically) filename = 'evil"\r\nX-Injected: yes'
# Download it and inspect raw response headers — no injected header should appear.

Related Issues

Discovered during a security audit of the repository. No CVE assigned (local network tool, not publicly exposed).

Checklist

  • Fixes do not introduce new dependencies
  • Existing functionality (file serving, directory listing, root index) is preserved
  • All user-controlled values are escaped before HTML/header output
  • Path resolution uses realpath() consistently
  • Traversal attempts are logged as warnings for visibility

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant