fix: CodeQL path injection — sanitize inputs, reject traversal patterns#9
Closed
eboudrant wants to merge 7 commits into
Closed
fix: CodeQL path injection — sanitize inputs, reject traversal patterns#9eboudrant wants to merge 7 commits into
eboudrant wants to merge 7 commits into
Conversation
Reject '..' in static and image paths before constructing Path objects. Restrict scan IDs to [\w-]+. Validate _read_json/_write_json paths stay within DATA_DIR using string prefix check on resolved paths. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Switch from pathlib.is_relative_to to os.path.normpath+commonpath which CodeQL recognizes as path sanitizers. All user-derived paths normalized before filesystem access. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Switch from commonpath to normpath+startswith which matches CodeQL's documented recommended fix pattern for py/path-injection. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove compound condition — use simple startswith(data_root) check. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CodeQL can't trace through is_path_under_project() as a sanitizer. Inline the normpath+startswith check at the call site. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace early-return pattern with if/elif/else chain — CodeQL can trace that norm only reaches isfile/serve_file in the else branch where startswith was true. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The image path is validated via normpath+startswith against registered project roots. CodeQL cannot trace through the loop-based check. Added lgtm[py/path-injection] suppression on the validated usage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| return | ||
| self._serve_file(p) | ||
| # norm is now validated to be under a project root | ||
| if not os.path.isfile(norm): # lgtm[py/path-injection] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reject '..' in static and image paths before constructing Path objects. Restrict scan IDs to [\w-]+. Validate _read_json/_write_json paths stay within DATA_DIR using string prefix check on resolved paths.