Fix code scanning alert no. 1: Uncontrolled data used in path expression #166
Conversation
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -38,7 +38,10 @@ | |||
| try: | |||
| return DotTemplate.read(glob()[stem]) | |||
| except KeyError: | |||
| if (root / stem).is_dir(): | |||
| fullpath = (root / stem).resolve() | |||
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 2 months ago
To fix the problem, we need to ensure that the constructed file path is properly normalized before checking if it is within the root directory. This involves using os.path.normpath to normalize the path and then verifying that the normalized path starts with the root directory.
- Normalize the constructed file path using
os.path.normpath. - Check if the normalized path starts with the root directory.
- If the path is not within the root directory, raise an exception.
| @@ -1,2 +1,3 @@ | ||
| from pathlib import Path | ||
| import os | ||
| from re import sub | ||
| @@ -41,2 +42,3 @@ | ||
| fullpath = (root / stem).resolve() | ||
| fullpath = Path(os.path.normpath(fullpath)) | ||
| if not str(fullpath).startswith(str(root.resolve())): |
There was a problem hiding this comment.
No. os.path is not recommended to be used in modern python projects.
| fullpath = (root / stem).resolve() | ||
| if not str(fullpath).startswith(str(root.resolve())): | ||
| raise Exception("Access to the path is not allowed") | ||
| if fullpath.is_dir(): |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix AI 2 months ago
To fix the problem, we need to ensure that the constructed file path is securely validated before being used. This involves normalizing the path and ensuring it is contained within a safe root directory. We will use os.path.normpath to normalize the path and then check that the normalized path starts with the root directory.
- Normalize the path using
os.path.normpath. - Ensure that the normalized path starts with the root directory.
- Update the
_load_templatefunction insrc/utils/load.pyto include these checks.
| @@ -1,2 +1,3 @@ | ||
| from pathlib import Path | ||
| import os | ||
| from re import sub | ||
| @@ -40,6 +41,6 @@ | ||
| except KeyError: | ||
| fullpath = (root / stem).resolve() | ||
| if not str(fullpath).startswith(str(root.resolve())): | ||
| fullpath = os.path.normpath((root / stem).resolve()) | ||
| if not fullpath.startswith(str(root.resolve())): | ||
| raise Exception("Access to the path is not allowed") | ||
| if fullpath.is_dir(): | ||
| if Path(fullpath).is_dir(): | ||
| return getattr(components, stem) |
Fixes https://github.com/promplate/demo/security/code-scanning/1
Fixes #119
To fix the problem, we need to ensure that the constructed file path remains within a safe root directory. This can be achieved by normalizing the path and checking that it starts with the root directory. We will use
os.path.normpathto normalize the path and then verify that the resulting path starts with the root directory.Suggested fixes powered by Copilot Autofix. Review carefully before merging.