Skip to content

Potential fix for code scanning alert no. 36: Uncontrolled data used in path expression#31

Open
akadata wants to merge 1 commit intomainfrom
alert-autofix-36
Open

Potential fix for code scanning alert no. 36: Uncontrolled data used in path expression#31
akadata wants to merge 1 commit intomainfrom
alert-autofix-36

Conversation

@akadata
Copy link
Owner

@akadata akadata commented Feb 18, 2026

Potential fix for https://github.com/akadata/pistorm64/security/code-scanning/36

In general, file paths derived from user-controlled data should be normalized/canonicalized and then checked to ensure they are within an expected root directory before any filesystem operation such as exists() or passing them into other components. This prevents directory traversal via .. components, symlinks, or absolute paths.

For this specific code, the best fix without changing visible behavior is:

  • Convert the user-supplied path into an absolute, resolved path under the configured image directory (self.cfg["dir"]), using Path.resolve(strict=False) or os.path.realpath.
  • Reject any path that is not within that directory using a robust check such as is_relative_to (Python 3.9+) or a manual relative_to/exception pattern.
  • Use the normalized path (p) for both the exists() check and constructing the insert command, instead of the raw, potentially relative/oddly-encoded user input.

Concretely, in do_POST under the /api/insert branch (around lines 1696–1716 in bin/adfmanager.py):

  1. After obtaining path = param("path", "") and decoding, compute a base directory base_dir = Path(self.cfg["dir"]).resolve() and create p as base_dir.joinpath(path).resolve(strict=False) if path is relative. If path is absolute, resolve it directly and then check that it lies under base_dir.
  2. Replace the existing p = Path(path) line and subsequent p.exists()/path_within_dir usage with logic that:
    • Normalizes to full_path = p.resolve(strict=False) (or equivalent).
    • Checks full_path.is_file() (or exists() as currently) only after verifying it is under base_dir.
    • Uses full_path in the insert command.
  3. Keep the existing error semantics (400 on not found, 400 on “must be inside image directory”) so external behavior remains the same, but with stronger guarantees against traversal and malicious absolute paths.

No new external dependencies are needed; we can do this with pathlib.Path, already imported.

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

…in path expression

Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@akadata akadata marked this pull request as ready for review February 18, 2026 15:50
base_dir = Path(self.cfg["dir"]).resolve()
raw_path = Path(path)
if not raw_path.is_absolute():
p = (base_dir / raw_path).resolve(strict=False)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
if not raw_path.is_absolute():
p = (base_dir / raw_path).resolve(strict=False)
else:
p = raw_path.resolve(strict=False)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
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