Potential fix for code scanning alert no. 16: Uncontrolled data used in path expression

[kimocoder]

Potential fix for https://github.com/kimocoder/wifite2/security/code-scanning/16

In general, the fix is to normalize the constructed file path and ensure it is strictly contained within the intended static root directory before using it, using a robust comparison (for example, os.path.commonpath or a normalized prefix comparison). This avoids directory traversal where attackers try inputs like /static/../../../../etc/passwd or attempt to exploit symlinks.

For this specific code, the best minimally invasive fix is:

• Normalize static_dir to an absolute, normalized path once in _serve_static_file.
• Compute full_path using os.path.join(static_dir, file_path) as before.
• Normalize full_path with os.path.realpath (or at least os.path.normpath) to collapse .. and follow symlinks.
• Replace the startswith check with a safer check using os.path.commonpath([static_dir, full_path]) == static_dir. This ensures full_path is inside static_dir, even if the attacker tries path traversal characters or absolute paths.
• Use the normalized full_path for the existence check and the file open.

Concretely, within wifite/attack/portal/server.py:

• In _serve_static_file, update how static_dir and full_path are computed.
• Replace the current check:

portal_dir = os.path.dirname(os.path.abspath(__file__))
static_dir = os.path.join(portal_dir, 'static')
full_path = os.path.join(static_dir, file_path)

# Security check: ensure file is within static directory
if not os.path.abspath(full_path).startswith(static_dir):
    self._send_error_response(403, 'Forbidden')
    return

with logic that:

• normalizes static_dir via os.path.realpath, and
• computes full_path = os.path.realpath(os.path.join(static_dir, file_path)),
• checks os.path.commonpath([static_dir, full_path]) == static_dir.

No new helper methods are strictly necessary; the existing imports already include os, so no additional imports are required. All other behavior (caching, content type selection, responses) remains unchanged.

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

[Copilot]

Pull request overview

This PR addresses a security vulnerability (code scanning alert #16) related to path traversal in the static file serving functionality. The fix replaces a simple startswith check with a more robust path validation approach using os.path.realpath() for normalization and os.path.commonpath() for containment verification. This prevents attackers from using path traversal techniques (e.g., ../../../etc/passwd) or symlinks to access files outside the intended static directory.

Changes:

• Normalize the static directory path using os.path.realpath() to resolve symlinks and relative paths
• Normalize the requested file path using os.path.realpath() to prevent path traversal attacks
• Replace the startswith security check with os.path.commonpath() for more robust path containment validation

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The os.path.commonpath function can raise a ValueError on Windows when paths are on different drives (e.g., C:\ and D:). While unlikely in this context (both paths should be on the same drive), this could cause the entire method to fail instead of gracefully returning a 403 Forbidden response. Consider wrapping the commonpath call in a try-except block or using an alternative approach like checking if the normalized full_path starts with the normalized static_dir followed by os.sep.

Suggested change

	if os.path.commonpath([static_dir, full_path]) != static_dir:
	self._send_error_response(403, 'Forbidden')
	return
	try:
	common_path = os.path.commonpath([static_dir, full_path])
	except ValueError:
	# Paths may be on different drives or otherwise incompatible; deny access.
	self._send_error_response(403, 'Forbidden')
	return
	if common_path != static_dir:
	self._send_error_response(403, 'Forbidden')
	return

[Copilot]

The security fix for path traversal lacks test coverage. Consider adding tests to verify that path traversal attempts (e.g., requests to '/static/../../../etc/passwd', '/static/..\..\..\windows\system32\config\sam') are properly blocked and return 403 Forbidden. This would help prevent regressions and validate the fix works correctly across different operating systems.