Skip to content

Python: use path-injection qhelp variant employed by autofix #21308

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tausbn merged 2 commits into from
Feb 12, 2026
+16 −10
Merged

Python: use path-injection qhelp variant employed by autofix #21308

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bed1ec8
Enhance path validation recommendations
smowton Feb 11, 2026
5f970d9
Rewordings per copilot
smowton Feb 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions python/ql/src/Security/CWE-022/PathInjection.qhelp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,27 @@ attacker being able to influence behavior by modifying unexpected files.

<recommendation>
<p>
Validate user input before using it to construct a file path, either using an off-the-shelf library function
like <code>werkzeug.utils.secure_filename</code>, or by performing custom validation.
Validate paths constructed from untrusted user input before using them to access files.
</p>

<p>
Ideally, follow these rules:
The choice of validation depends on the use case.
</p>

<ul>
<li>Do not allow more than a single "." character.</li>
<li>Do not allow directory separators such as "/" or "\" (depending on the file system).</li>
<li>Do not rely on simply replacing problematic sequences such as "../". For example, after
applying this filter to ".../...//", the resulting string would still be "../".</li>
<li>Use an allowlist of known good patterns.</li>
</ul>
<p>
If you want to allow paths spanning multiple folders, a common strategy is to make sure that the constructed
file path is contained within a safe root folder. First, normalize the path using <code>os.path.normpath</code> or
<code>os.path.realpath</code> (make sure to use the latter if symlinks are a consideration)
to remove any internal ".." segments and/or follow links. Then check that the normalized path starts with the
root folder. Note that the normalization step is important, since otherwise even a path that starts with the root
folder could be used to access files outside the root folder.
Comment on lines 25 to 29
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saying that os.path.normpath or os.path.realpath can be used to "remove any '..' segments" is misleading from a security perspective: normpath is purely lexical (doesn't make the path absolute or resolve symlinks) and can give a false sense of safety against traversal via symlinks within the root. Consider recommending resolving both root and the constructed path (for example with realpath/Path.resolve) before performing the containment check, and clarifying the symlink caveat if you keep normpath as an option.

This issue also appears on line 24 of the same file.

Suggested change
file path is contained within a safe root folder. First, normalize the path using <code>os.path.normpath</code> or
<code>os.path.realpath</code> to remove any ".." segments. Then check that the normalized path starts with the
root folder. Note that the normalization step is important, since otherwise even a path that starts with the root
folder could be used to access files outside the root folder.
file path is contained within a safe root folder. To do this safely, resolve both the root folder and the
constructed path to absolute, symlink-resolved paths (for example, using <code>os.path.realpath</code> or
<code>pathlib.Path.resolve</code>), and then check that the resolved path is contained within the resolved root.
Note that <code>os.path.normpath</code> only performs lexical normalization (for example, collapsing ".."
segments) and does not resolve symlinks, so it is not sufficient on its own to prevent traversal via symlinks
inside the root, although it can be used as a preliminary cleanup step.

Copilot uses AI. Check for mistakes.
</p>

<p>
More restrictive options include using a library function like <code>werkzeug.utils.secure_filename</code> to eliminate
any special characters from the file path, or restricting the path to a known list of safe paths. These options are
safe, but can only be used in particular circumstances.
Comment on lines 33 to 35
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Terminology consistency: elsewhere in the Python qhelp docs the term “allowlist” is used (for example python/ql/src/Security/CWE-078/CommandInjection.qhelp and python/ql/src/Security/CWE-020/IncompleteUrlSubstringSanitization.qhelp). Consider using “allowlist” here as well instead of “allow list”.

Copilot uses AI. Check for mistakes.
</p>
</recommendation>

<example>
Expand Down