Skip to content
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

Fix code scanning alert no. 1: Uncontrolled data used in path expression #166

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Fix code scanning alert no. 1: Uncontrolled data used in path expression #166

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 4 additions & 1 deletion src/utils/load.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

This path depends on a
user-provided value
Loading
.

Copilot Autofix AI 3 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.

  1. Normalize the constructed file path using os.path.normpath.
  2. Check if the normalized path starts with the root directory.
  3. If the path is not within the root directory, raise an exception.
Suggested changeset 1
src/utils/load.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/utils/load.py b/src/utils/load.py
--- a/src/utils/load.py
+++ b/src/utils/load.py
@@ -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())):
EOF
@@ -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())):
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Copy link
Member Author

Choose a reason for hiding this comment

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

No. os.path is not recommended to be used in modern python projects.

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

This path depends on a
user-provided value
Loading
.

Copilot Autofix AI 3 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.

  1. Normalize the path using os.path.normpath.
  2. Ensure that the normalized path starts with the root directory.
  3. Update the _load_template function in src/utils/load.py to include these checks.
Suggested changeset 1
src/utils/load.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/utils/load.py b/src/utils/load.py
--- a/src/utils/load.py
+++ b/src/utils/load.py
@@ -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)
EOF
@@ -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)
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
return getattr(components, stem)
raise

Expand Down