Closed
Conversation
This commit deletes the entire `githubapp` module, including `events`, `test_helper`, and configuration files, as well as classes for handling GitHub events and check runs. This likely indicates the functionality has been deprecated, migrated, or replaced.
|
Here's the code health analysis summary for commits Analysis Summary
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| print("Saving in", file_name, end="...", flush=True) | ||
| data = clean_data(data) | ||
| data["installation"]["id"] = 12345678 | ||
| with open(file_name, "w") as file: |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the problem, we need to ensure that the constructed file path cannot escape the intended payloads directory (PAYLOADS_FOLDER). The best way to do this is to:
- Sanitize the
event_full_nameso that it cannot contain path traversal characters or other problematic input. The most robust way is to usewerkzeug.utils.secure_filename, which strips out dangerous characters and ensures the filename is safe. - Alternatively, and in addition, after constructing the full path, normalize it with
os.path.normpathand check that it still resides withinPAYLOADS_FOLDER(i.e., the normalized path starts with the absolute path ofPAYLOADS_FOLDER).
Since we are only allowed to use well-known libraries, and werkzeug.utils.secure_filename is a standard tool for this in Flask apps, we will use it. We will also add an import for secure_filename at the top of the file.
Changes to make:
- Add
from werkzeug.utils import secure_filenameto the imports. - When constructing
file_name, usesecure_filename(event_full_name)instead of the raw value. - Optionally, for extra defense, after joining and normalizing the path, check that it is still within
PAYLOADS_FOLDER.
Suggested changeset
1
payload_helper.py
| @@ -7,2 +7,3 @@ | ||
| from flask import request, Flask | ||
| from werkzeug.utils import secure_filename | ||
| from pcommand import ArgumentParser, command | ||
| @@ -122,3 +123,4 @@ | ||
| print() | ||
| file_name = f"{PAYLOADS_FOLDER}/{event_full_name}.json" | ||
| safe_event_full_name = secure_filename(event_full_name) | ||
| file_name = f"{PAYLOADS_FOLDER}/{safe_event_full_name}.json" | ||
| print("Saving in", file_name, end="...", flush=True) |
Copilot is powered by AI and may make mistakes. Always verify output.
There was a problem hiding this comment.
Micro-Learning Topic: Uncontrolled data used in path expression (Detected by phrase)
Matched on "Uncontrolled data used in path expression"
Accessing paths influenced by users can allow an attacker to access unexpected resources.
Try a challenge in Secure Code Warrior
Helpful references
- OWASP Input Validation Cheat Sheet - This cheatsheet is focused on providing clear, simple, actionable guidance for preventing injection and input validation flaws in your applications, including defence against path traversal.
- OWASP Path Traversal - OWASP community page with comprehensive information about path traversal, and links to various OWASP resources to help detect or prevent it.
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
1 participant
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.
Pull Request automatically created