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.

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

4.2.0 #2450

Merged
merged 16 commits into from
Nov 18, 2024
Merged

4.2.0 #2450

merged 16 commits into from
Nov 18, 2024

Conversation

ajinabraham
Copy link
Member

@ajinabraham ajinabraham commented Nov 12, 2024

Describe the Pull Request

  • Added malware lookup using SHA2 with VirusTotal, Triage, Hybrid Analysis, and MetaDefender.
  • Fixed permissions of extracted files to counter anti-analysis techniques.
  • Resolved APK parsing errors in androguard.
  • Handled exceptions in string_on_binary.
  • Optimized APK ZIP analysis for improved performance.
  • Fixed untar permission errors in dynamic analysis.
  • Added bypass for SSL pinning in Boye's AbstractVerifier.
  • Updated bypass for SSL pinning in Appmattus's CertificateTransparencyInterceptor.
  • Introduced SSL pinning detector script.
  • Improved Frida intent dumper script.
  • Added Frida intent tracer script.
  • Introduced timeouts for all HTTP calls.
  • Added django-q2-based asynchronous scans for Android and iOS binaries and source code.
  • Fixed bug in certificate analysis.
  • Enabled asynchronous scans in Docker Compose setup.
  • Performed QA for Android and iOS SAST modules.
  • Added Frida script for audit-webview.
  • Introduced Frida script for trace-javascript-interface.
  • Upgraded libsast for improved file reading, multiprocessing, and multithreading.
  • Fixed PNG crush issues on Darwin systems.
  • Performed QA on the home screen UI.
  • Updated httptools and libsast dependencies.

Checklist for PR

  • Run MobSF unit tests and lint tox -e lint,test
  • Tested Working on Linux, Mac, Windows, and Docker
  • Add unit test for any new Web API (Refer: StaticAnalyzer/tests.py)
  • Make sure tests are passing on your PR MobSF tests

Additional Comments (if any)

DESCRIBE HERE

Copy link

👋 @ajinabraham
Thank you for sending this pull request ❤️.
Please make sure you have followed our contribution guidelines. We will review it as soon as possible

@ajinabraham ajinabraham changed the title Added Malware lookup for Android, iOS, Windows and other binary types 4.2.0 Nov 12, 2024
perm_755 = stat.S_IRWXU | stat.S_IRGRP | stat.S_IXGRP | stat.S_IROTH | stat.S_IXOTH
perm_644 = stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH
# Set permissions for directories and files
for item in base_path.rglob('*'):

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2). This path depends on a [user-provided value](3). This path depends on a [user-provided value](4).

Copilot Autofix AI about 2 months ago

To fix the problem, we need to ensure that the constructed file path is contained within a safe root folder. This can be achieved by normalizing the path using os.path.normpath and then checking that the normalized path starts with the root folder. This approach will prevent path traversal attacks by ensuring that the path does not escape the intended directory.

  1. Normalize the path using os.path.normpath.
  2. Check that the normalized path starts with the intended base directory.
  3. If the check fails, raise an exception or handle the error appropriately.
Suggested changeset 1
mobsf/MobSF/utils.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/mobsf/MobSF/utils.py b/mobsf/MobSF/utils.py
--- a/mobsf/MobSF/utils.py
+++ b/mobsf/MobSF/utils.py
@@ -965,2 +965,6 @@
     base_path = Path(path)
+    base_dir = Path(settings.BASE_DIR)
+    normalized_path = base_path.resolve()
+    if not str(normalized_path).startswith(str(base_dir)):
+        raise Exception("Path traversal attempt detected")
     perm_755 = stat.S_IRWXU | stat.S_IRGRP | stat.S_IXGRP | stat.S_IROTH | stat.S_IXOTH
@@ -968,3 +972,3 @@
     # Set permissions for directories and files
-    for item in base_path.rglob('*'):
+    for item in normalized_path.rglob('*'):
         try:
EOF
@@ -965,2 +965,6 @@
base_path = Path(path)
base_dir = Path(settings.BASE_DIR)
normalized_path = base_path.resolve()
if not str(normalized_path).startswith(str(base_dir)):
raise Exception("Path traversal attempt detected")
perm_755 = stat.S_IRWXU | stat.S_IRGRP | stat.S_IXGRP | stat.S_IROTH | stat.S_IXOTH
@@ -968,3 +972,3 @@
# Set permissions for directories and files
for item in base_path.rglob('*'):
for item in normalized_path.rglob('*'):
try:
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
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
mobsf/StaticAnalyzer/views/android/apk.py Dismissed Show dismissed Hide dismissed
mobsf/StaticAnalyzer/views/android/apk.py Fixed Show fixed Hide fixed
mobsf/StaticAnalyzer/views/android/apk.py Fixed Show fixed Hide fixed
mobsf/StaticAnalyzer/views/android/apk.py Fixed Show fixed Hide fixed
mobsf/StaticAnalyzer/views/android/apk.py Fixed Show fixed Hide fixed
mobsf/StaticAnalyzer/views/android/apk.py Fixed Show fixed Hide fixed
mobsf/StaticAnalyzer/views/android/apk.py Fixed Show fixed Hide fixed
mobsf/StaticAnalyzer/views/android/apk.py Fixed Show fixed Hide fixed
mobsf/StaticAnalyzer/views/android/apk.py Fixed Show fixed Hide fixed
mobsf/StaticAnalyzer/views/android/apk.py Fixed Show fixed Hide fixed
context['appsec'] = get_android_dashboard(context, True)
context['average_cvss'] = get_avg_cvss(context['code_analysis'])
logcat_file = Path(app_dic['app_dir']) / 'logcat.txt'
context['dynamic_analysis_done'] = logcat_file.exists()

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2).

Copilot Autofix AI about 2 months ago

To fix the problem, we need to ensure that the constructed file path is contained within a safe root folder. We can achieve this by normalizing the path using os.path.normpath and then checking that the normalized path starts with the root folder. This will prevent any path traversal attacks.

  1. Normalize the logcat_file path using os.path.normpath.
  2. Check that the normalized path starts with the expected base directory (app_dic['app_dir']).
  3. If the check fails, raise an exception or handle the error appropriately.
Suggested changeset 1
mobsf/StaticAnalyzer/views/android/apk.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/mobsf/StaticAnalyzer/views/android/apk.py b/mobsf/StaticAnalyzer/views/android/apk.py
--- a/mobsf/StaticAnalyzer/views/android/apk.py
+++ b/mobsf/StaticAnalyzer/views/android/apk.py
@@ -5,2 +5,3 @@
 from pathlib import Path
+import os
 
@@ -263,3 +264,6 @@
     logcat_file = Path(app_dic['app_dir']) / 'logcat.txt'
-    context['dynamic_analysis_done'] = logcat_file.exists()
+    normalized_logcat_file = Path(os.path.normpath(logcat_file))
+    if not str(normalized_logcat_file).startswith(str(Path(app_dic['app_dir']).resolve())):
+        raise Exception("Invalid logcat file path")
+    context['dynamic_analysis_done'] = normalized_logcat_file.exists()
     context['virus_total'] = None
EOF
@@ -5,2 +5,3 @@
from pathlib import Path
import os

@@ -263,3 +264,6 @@
logcat_file = Path(app_dic['app_dir']) / 'logcat.txt'
context['dynamic_analysis_done'] = logcat_file.exists()
normalized_logcat_file = Path(os.path.normpath(logcat_file))
if not str(normalized_logcat_file).startswith(str(Path(app_dic['app_dir']).resolve())):
raise Exception("Invalid logcat file path")
context['dynamic_analysis_done'] = normalized_logcat_file.exists()
context['virus_total'] = None
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
# Eclipse
man = app_path / 'AndroidManifest.xml'
src = app_path / 'src'
if man.is_file() and src.exists():

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2).

Copilot Autofix AI about 2 months ago

To fix the problem, we need to ensure that the constructed file paths are contained within a safe root directory. This can be achieved by normalizing the path and verifying that it starts with the expected base directory. Specifically, we will:

  1. Normalize the app_path using os.path.normpath.
  2. Check that the normalized path starts with the expected base directory.
Suggested changeset 1
mobsf/StaticAnalyzer/views/android/apk.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/mobsf/StaticAnalyzer/views/android/apk.py b/mobsf/StaticAnalyzer/views/android/apk.py
--- a/mobsf/StaticAnalyzer/views/android/apk.py
+++ b/mobsf/StaticAnalyzer/views/android/apk.py
@@ -456,4 +456,9 @@
 
-def is_android_source(app_path):
+def is_android_source(app_path, base_dir):
     """Detect Android Source and IDE Type."""
+    # Normalize and validate app_path
+    app_path = Path(os.path.normpath(app_path))
+    if not str(app_path).startswith(str(base_dir)):
+        raise Exception("Invalid app path")
+
     # Eclipse
@@ -489,3 +494,4 @@
         app_path = Path(app_dir)
-        ide, is_and = is_android_source(app_path)
+        base_dir = Path(settings.UPLD_DIR)
+        ide, is_and = is_android_source(app_path, base_dir)
 
@@ -497,3 +503,3 @@
             if subdir.is_dir() and subdir.exists():
-                ide, is_and = is_android_source(subdir)
+                ide, is_and = is_android_source(subdir, base_dir)
                 if ide:
EOF
@@ -456,4 +456,9 @@

def is_android_source(app_path):
def is_android_source(app_path, base_dir):
"""Detect Android Source and IDE Type."""
# Normalize and validate app_path
app_path = Path(os.path.normpath(app_path))
if not str(app_path).startswith(str(base_dir)):
raise Exception("Invalid app path")

# Eclipse
@@ -489,3 +494,4 @@
app_path = Path(app_dir)
ide, is_and = is_android_source(app_path)
base_dir = Path(settings.UPLD_DIR)
ide, is_and = is_android_source(app_path, base_dir)

@@ -497,3 +503,3 @@
if subdir.is_dir() and subdir.exists():
ide, is_and = is_android_source(subdir)
ide, is_and = is_android_source(subdir, base_dir)
if ide:
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
# Eclipse
man = app_path / 'AndroidManifest.xml'
src = app_path / 'src'
if man.is_file() and src.exists():

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2).

Copilot Autofix AI about 2 months ago

To fix the problem, we need to ensure that the constructed file path is contained within a safe root folder. We will normalize the path using os.path.normpath to remove any ".." segments and then check that the normalized path starts with the root folder. This will prevent path traversal attacks.

  1. Normalize the app_path using os.path.normpath.
  2. Check that the normalized app_path starts with the predefined base directory (settings.UPLD_DIR).
  3. If the check fails, raise an exception or handle the error appropriately.
Suggested changeset 1
mobsf/StaticAnalyzer/views/android/apk.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/mobsf/StaticAnalyzer/views/android/apk.py b/mobsf/StaticAnalyzer/views/android/apk.py
--- a/mobsf/StaticAnalyzer/views/android/apk.py
+++ b/mobsf/StaticAnalyzer/views/android/apk.py
@@ -5,2 +5,3 @@
 from pathlib import Path
+import os
 
@@ -488,3 +489,6 @@
 
-        app_path = Path(app_dir)
+        app_path = Path(os.path.normpath(app_dir))
+        base_dir = Path(settings.UPLD_DIR).resolve()
+        if not str(app_path.resolve()).startswith(str(base_dir)):
+            raise Exception("Invalid app directory path")
         ide, is_and = is_android_source(app_path)
EOF
@@ -5,2 +5,3 @@
from pathlib import Path
import os

@@ -488,3 +489,6 @@

app_path = Path(app_dir)
app_path = Path(os.path.normpath(app_dir))
base_dir = Path(settings.UPLD_DIR).resolve()
if not str(app_path.resolve()).startswith(str(base_dir)):
raise Exception("Invalid app directory path")
ide, is_and = is_android_source(app_path)
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
man = app_path / 'app' / 'src' / 'main' / 'AndroidManifest.xml'
java = app_path / 'app' / 'src' / 'main' / 'java'
kotlin = app_path / 'app' / 'src' / 'main' / 'kotlin'
if man.is_file() and (java.exists() or kotlin.exists()):

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2).

Copilot Autofix AI about 2 months ago

To fix the problem, we need to ensure that the constructed file paths are contained within a safe root directory. This can be achieved by normalizing the path using os.path.normpath and then checking that the normalized path starts with the intended base directory. This approach will prevent directory traversal attacks by ensuring that any ".." segments are resolved and the final path is within the allowed directory.

  1. Normalize the constructed path using os.path.normpath.
  2. Check that the normalized path starts with the base directory.
  3. Raise an exception or handle the error if the path is not within the allowed directory.
Suggested changeset 1
mobsf/StaticAnalyzer/views/android/apk.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/mobsf/StaticAnalyzer/views/android/apk.py b/mobsf/StaticAnalyzer/views/android/apk.py
--- a/mobsf/StaticAnalyzer/views/android/apk.py
+++ b/mobsf/StaticAnalyzer/views/android/apk.py
@@ -456,4 +456,11 @@
 
-def is_android_source(app_path):
+def is_android_source(app_path, base_dir):
     """Detect Android Source and IDE Type."""
+    def is_within_base_dir(path, base):
+        return os.path.commonpath([path]) == os.path.commonpath([path, base])
+
+    # Normalize paths
+    app_path = Path(os.path.normpath(app_path))
+    base_dir = Path(os.path.normpath(base_dir))
+
     # Eclipse
@@ -461,3 +468,3 @@
     src = app_path / 'src'
-    if man.is_file() and src.exists():
+    if is_within_base_dir(man, base_dir) and man.is_file() and src.exists():
         return 'eclipse', True
@@ -468,3 +475,3 @@
     kotlin = app_path / 'app' / 'src' / 'main' / 'kotlin'
-    if man.is_file() and (java.exists() or kotlin.exists()):
+    if is_within_base_dir(man, base_dir) and man.is_file() and (java.exists() or kotlin.exists()):
         return 'studio', True
@@ -481,3 +488,3 @@
 
-def valid_source_code(checksum, app_dir):
+def valid_source_code(checksum, app_dir, base_dir):
     """Test if this is a valid source code zip."""
@@ -489,3 +496,3 @@
         app_path = Path(app_dir)
-        ide, is_and = is_android_source(app_path)
+        ide, is_and = is_android_source(app_path, base_dir)
 
@@ -497,3 +504,3 @@
             if subdir.is_dir() and subdir.exists():
-                ide, is_and = is_android_source(subdir)
+                ide, is_and = is_android_source(subdir, base_dir)
                 if ide:
EOF
@@ -456,4 +456,11 @@

def is_android_source(app_path):
def is_android_source(app_path, base_dir):
"""Detect Android Source and IDE Type."""
def is_within_base_dir(path, base):
return os.path.commonpath([path]) == os.path.commonpath([path, base])

# Normalize paths
app_path = Path(os.path.normpath(app_path))
base_dir = Path(os.path.normpath(base_dir))

# Eclipse
@@ -461,3 +468,3 @@
src = app_path / 'src'
if man.is_file() and src.exists():
if is_within_base_dir(man, base_dir) and man.is_file() and src.exists():
return 'eclipse', True
@@ -468,3 +475,3 @@
kotlin = app_path / 'app' / 'src' / 'main' / 'kotlin'
if man.is_file() and (java.exists() or kotlin.exists()):
if is_within_base_dir(man, base_dir) and man.is_file() and (java.exists() or kotlin.exists()):
return 'studio', True
@@ -481,3 +488,3 @@

def valid_source_code(checksum, app_dir):
def valid_source_code(checksum, app_dir, base_dir):
"""Test if this is a valid source code zip."""
@@ -489,3 +496,3 @@
app_path = Path(app_dir)
ide, is_and = is_android_source(app_path)
ide, is_and = is_android_source(app_path, base_dir)

@@ -497,3 +504,3 @@
if subdir.is_dir() and subdir.exists():
ide, is_and = is_android_source(subdir)
ide, is_and = is_android_source(subdir, base_dir)
if ide:
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
man = app_path / 'app' / 'src' / 'main' / 'AndroidManifest.xml'
java = app_path / 'app' / 'src' / 'main' / 'java'
kotlin = app_path / 'app' / 'src' / 'main' / 'kotlin'
if man.is_file() and (java.exists() or kotlin.exists()):

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2).

Copilot Autofix AI about 2 months ago

To fix the problem, we need to ensure that the constructed file path is contained within a safe root folder. We can achieve this by normalizing the path using os.path.normpath and then checking that the normalized path starts with the root folder. This will prevent any path traversal attempts.

  1. Normalize the app_path using os.path.normpath.
  2. Check that the normalized app_path starts with the intended root directory (settings.UPLD_DIR).
  3. If the check fails, raise an exception or handle the error appropriately.
Suggested changeset 1
mobsf/StaticAnalyzer/views/android/apk.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/mobsf/StaticAnalyzer/views/android/apk.py b/mobsf/StaticAnalyzer/views/android/apk.py
--- a/mobsf/StaticAnalyzer/views/android/apk.py
+++ b/mobsf/StaticAnalyzer/views/android/apk.py
@@ -5,2 +5,3 @@
 from pathlib import Path
+import os
 
@@ -488,3 +489,6 @@
 
-        app_path = Path(app_dir)
+        app_path = Path(os.path.normpath(app_dir))
+        root_dir = Path(settings.UPLD_DIR).resolve()
+        if not str(app_path.resolve()).startswith(str(root_dir)):
+            raise Exception("Invalid app directory path")
         ide, is_and = is_android_source(app_path)
EOF
@@ -5,2 +5,3 @@
from pathlib import Path
import os

@@ -488,3 +489,6 @@

app_path = Path(app_dir)
app_path = Path(os.path.normpath(app_dir))
root_dir = Path(settings.UPLD_DIR).resolve()
if not str(app_path.resolve()).startswith(str(root_dir)):
raise Exception("Invalid app directory path")
ide, is_and = is_android_source(app_path)
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
man = app_path / 'app' / 'src' / 'main' / 'AndroidManifest.xml'
java = app_path / 'app' / 'src' / 'main' / 'java'
kotlin = app_path / 'app' / 'src' / 'main' / 'kotlin'
if man.is_file() and (java.exists() or kotlin.exists()):

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2).

Copilot Autofix AI about 2 months ago

To fix the problem, we need to ensure that the constructed file paths are contained within a safe root folder. We can achieve this by normalizing the path using os.path.normpath and then checking that the normalized path starts with the root folder. This will prevent any path traversal attacks.

  1. Normalize the constructed paths using os.path.normpath.
  2. Check that the normalized paths start with the expected root folder.
  3. Raise an exception if the paths are not within the expected root folder.
Suggested changeset 1
mobsf/StaticAnalyzer/views/android/apk.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/mobsf/StaticAnalyzer/views/android/apk.py b/mobsf/StaticAnalyzer/views/android/apk.py
--- a/mobsf/StaticAnalyzer/views/android/apk.py
+++ b/mobsf/StaticAnalyzer/views/android/apk.py
@@ -456,4 +456,12 @@
 
-def is_android_source(app_path):
+def is_android_source(app_path, root_path):
     """Detect Android Source and IDE Type."""
+    # Normalize paths
+    app_path = Path(os.path.normpath(app_path))
+    root_path = Path(os.path.normpath(root_path))
+
+    # Ensure paths are within the root directory
+    if not str(app_path).startswith(str(root_path)):
+        raise Exception("Path traversal detected")
+
     # Eclipse
@@ -489,3 +497,3 @@
         app_path = Path(app_dir)
-        ide, is_and = is_android_source(app_path)
+        ide, is_and = is_android_source(app_path, Path(settings.UPLD_DIR))
 
EOF
@@ -456,4 +456,12 @@

def is_android_source(app_path):
def is_android_source(app_path, root_path):
"""Detect Android Source and IDE Type."""
# Normalize paths
app_path = Path(os.path.normpath(app_path))
root_path = Path(os.path.normpath(root_path))

# Ensure paths are within the root directory
if not str(app_path).startswith(str(root_path)):
raise Exception("Path traversal detected")

# Eclipse
@@ -489,3 +497,3 @@
app_path = Path(app_dir)
ide, is_and = is_android_source(app_path)
ide, is_and = is_android_source(app_path, Path(settings.UPLD_DIR))

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
def move_to_parent(inside_path, app_path):
"""Move contents of inside to app dir."""
for item in inside_path.iterdir():
shutil.move(str(item), str(app_path))

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2).

Copilot Autofix AI about 2 months ago

To fix the problem, we need to ensure that the constructed file path is contained within a safe root folder. We will normalize the path using os.path.normpath and then check that the normalized path starts with the root folder. This will prevent directory traversal attacks and ensure that the file operations are performed within the intended directory.

  1. Normalize the app_path using os.path.normpath.
  2. Check that the normalized path starts with the expected base directory.
  3. If the check fails, raise an exception or handle the error appropriately.
Suggested changeset 1
mobsf/StaticAnalyzer/views/android/apk.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/mobsf/StaticAnalyzer/views/android/apk.py b/mobsf/StaticAnalyzer/views/android/apk.py
--- a/mobsf/StaticAnalyzer/views/android/apk.py
+++ b/mobsf/StaticAnalyzer/views/android/apk.py
@@ -476,2 +476,6 @@
     """Move contents of inside to app dir."""
+    app_path = Path(os.path.normpath(app_path))
+    base_dir = Path(settings.UPLD_DIR).resolve()
+    if not str(app_path).startswith(str(base_dir)):
+        raise Exception("Invalid path: outside of allowed directory")
     for item in inside_path.iterdir():
EOF
@@ -476,2 +476,6 @@
"""Move contents of inside to app dir."""
app_path = Path(os.path.normpath(app_path))
base_dir = Path(settings.UPLD_DIR).resolve()
if not str(app_path).startswith(str(base_dir)):
raise Exception("Invalid path: outside of allowed directory")
for item in inside_path.iterdir():
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
_, exc, _ = exc_info
if exc.errno == errno.EACCES: # Permission error
try:
os.chmod(path, 0o777)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2).

Copilot Autofix AI about 2 months ago

To fix the problem, we need to ensure that the path variable used in the onerror function is validated to prevent path traversal attacks. We can achieve this by normalizing the path and ensuring it is within a predefined safe directory.

  1. Normalize the path using os.path.normpath to remove any ".." segments.
  2. Check that the normalized path starts with the root directory of the application data.
Suggested changeset 1
mobsf/DynamicAnalyzer/views/common/shared.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/mobsf/DynamicAnalyzer/views/common/shared.py b/mobsf/DynamicAnalyzer/views/common/shared.py
--- a/mobsf/DynamicAnalyzer/views/common/shared.py
+++ b/mobsf/DynamicAnalyzer/views/common/shared.py
@@ -59,6 +59,11 @@
     _, exc, _ = exc_info
+    safe_root = os.path.abspath('/path/to/safe/root')  # Define the safe root directory
+    normalized_path = os.path.normpath(path)
+    if not normalized_path.startswith(safe_root):
+        logger.error('Unsafe path detected: %s', path)
+        return
     if exc.errno == errno.EACCES:  # Permission error
         try:
-            os.chmod(path, 0o777)
-            func(path)
+            os.chmod(normalized_path, 0o777)
+            func(normalized_path)
         except Exception:
@@ -67,3 +72,3 @@
         try:
-            func(path)
+            func(normalized_path)
         except Exception:
EOF
@@ -59,6 +59,11 @@
_, exc, _ = exc_info
safe_root = os.path.abspath('/path/to/safe/root') # Define the safe root directory
normalized_path = os.path.normpath(path)
if not normalized_path.startswith(safe_root):
logger.error('Unsafe path detected: %s', path)
return
if exc.errno == errno.EACCES: # Permission error
try:
os.chmod(path, 0o777)
func(path)
os.chmod(normalized_path, 0o777)
func(normalized_path)
except Exception:
@@ -67,3 +72,3 @@
try:
func(path)
func(normalized_path)
except Exception:
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
try:
# Extract Device Data
if not tar_loc.exists():

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2).

Copilot Autofix AI about 2 months ago

To fix the problem, we need to validate the tar_loc path before using it to open the tar file. We can achieve this by ensuring that the tar_loc path is contained within a safe root directory. This involves normalizing the path and checking that it starts with the expected base directory.

  1. Normalize the tar_loc path using os.path.normpath.
  2. Check that the normalized path starts with the expected base directory.
  3. Raise an exception if the path is not valid.
Suggested changeset 1
mobsf/DynamicAnalyzer/views/common/shared.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/mobsf/DynamicAnalyzer/views/common/shared.py b/mobsf/DynamicAnalyzer/views/common/shared.py
--- a/mobsf/DynamicAnalyzer/views/common/shared.py
+++ b/mobsf/DynamicAnalyzer/views/common/shared.py
@@ -79,2 +79,6 @@
         # Extract Device Data
+        base_path = os.path.abspath(settings.UPLD_DIR)
+        tar_loc = os.path.normpath(tar_loc)
+        if not tar_loc.startswith(base_path):
+            raise Exception('Invalid tar file location')
         if not tar_loc.exists():
EOF
@@ -79,2 +79,6 @@
# Extract Device Data
base_path = os.path.abspath(settings.UPLD_DIR)
tar_loc = os.path.normpath(tar_loc)
if not tar_loc.startswith(base_path):
raise Exception('Invalid tar file location')
if not tar_loc.exists():
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
# Extract Device Data
if not tar_loc.exists():
return False
if untar_dir.exists():

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2).

Copilot Autofix AI about 2 months ago

To fix the problem, we need to ensure that the untar_dir path is validated before it is used. We can achieve this by normalizing the path and checking that it is within a safe root directory. This approach will prevent directory traversal attacks by ensuring that the constructed path does not escape the intended directory.

  1. Normalize the untar_dir path using os.path.normpath.
  2. Check that the normalized path starts with the intended root directory.
  3. Raise an exception if the path is not within the intended directory.
Suggested changeset 1
mobsf/DynamicAnalyzer/views/common/shared.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/mobsf/DynamicAnalyzer/views/common/shared.py b/mobsf/DynamicAnalyzer/views/common/shared.py
--- a/mobsf/DynamicAnalyzer/views/common/shared.py
+++ b/mobsf/DynamicAnalyzer/views/common/shared.py
@@ -81,2 +81,6 @@
             return False
+        # Normalize and validate untar_dir
+        untar_dir = os.path.normpath(untar_dir)
+        if not untar_dir.startswith(os.path.abspath(Path(untar_dir).parent)):
+            raise Exception('Invalid untar_dir path')
         if untar_dir.exists():
EOF
@@ -81,2 +81,6 @@
return False
# Normalize and validate untar_dir
untar_dir = os.path.normpath(untar_dir)
if not untar_dir.startswith(os.path.abspath(Path(untar_dir).parent)):
raise Exception('Invalid untar_dir path')
if untar_dir.exists():
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 False
if untar_dir.exists():
# fix for permission errors
shutil.rmtree(untar_dir, onerror=onerror)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2).

Copilot Autofix AI about 2 months ago

To fix the problem, we need to ensure that the untar_dir path is validated before it is used. We can achieve this by normalizing the path and ensuring it is contained within a predefined safe directory. This will prevent any path traversal attacks.

  1. Normalize the untar_dir path using os.path.normpath.
  2. Check that the normalized path starts with a predefined safe directory.
  3. Raise an exception if the path is not within the safe directory.
Suggested changeset 1
mobsf/DynamicAnalyzer/views/common/shared.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/mobsf/DynamicAnalyzer/views/common/shared.py b/mobsf/DynamicAnalyzer/views/common/shared.py
--- a/mobsf/DynamicAnalyzer/views/common/shared.py
+++ b/mobsf/DynamicAnalyzer/views/common/shared.py
@@ -81,2 +81,7 @@
             return False
+        # Normalize and validate untar_dir
+        safe_base_dir = os.path.abspath('/safe/base/directory')
+        untar_dir = os.path.normpath(untar_dir)
+        if not untar_dir.startswith(safe_base_dir):
+            raise Exception('Invalid extraction directory')
         if untar_dir.exists():
EOF
@@ -81,2 +81,7 @@
return False
# Normalize and validate untar_dir
safe_base_dir = os.path.abspath('/safe/base/directory')
untar_dir = os.path.normpath(untar_dir)
if not untar_dir.startswith(safe_base_dir):
raise Exception('Invalid extraction directory')
if untar_dir.exists():
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
# fix for permission errors
shutil.rmtree(untar_dir, onerror=onerror)
else:
os.makedirs(untar_dir)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2).

Copilot Autofix AI about 2 months ago

To fix the problem, we need to ensure that the untar_dir path is validated before it is used to create directories or extract files. We can achieve this by normalizing the path and ensuring it is contained within a safe root directory. This approach will prevent directory traversal attacks by ensuring that the constructed path does not escape the intended directory.

  1. Normalize the untar_dir path using os.path.normpath.
  2. Check that the normalized path starts with the intended root directory.
  3. Raise an exception if the path validation fails.
Suggested changeset 1
mobsf/DynamicAnalyzer/views/common/shared.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/mobsf/DynamicAnalyzer/views/common/shared.py b/mobsf/DynamicAnalyzer/views/common/shared.py
--- a/mobsf/DynamicAnalyzer/views/common/shared.py
+++ b/mobsf/DynamicAnalyzer/views/common/shared.py
@@ -81,2 +81,7 @@
             return False
+        # Normalize and validate untar_dir
+        untar_dir = os.path.normpath(untar_dir)
+        safe_root = os.path.normpath('/path/to/safe/root')  # Define a safe root directory
+        if not untar_dir.startswith(safe_root):
+            raise Exception('Invalid extraction directory')
         if untar_dir.exists():
EOF
@@ -81,2 +81,7 @@
return False
# Normalize and validate untar_dir
untar_dir = os.path.normpath(untar_dir)
safe_root = os.path.normpath('/path/to/safe/root') # Define a safe root directory
if not untar_dir.startswith(safe_root):
raise Exception('Invalid extraction directory')
if untar_dir.exists():
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
mobsf/StaticAnalyzer/views/android/apk.py Dismissed Show dismissed Hide dismissed
@@ -62,7 +62,7 @@
sha1 = hashlib.sha1()
sha256 = hashlib.sha256()
block_size = 65536
with io.open(app_path, mode='rb') as afile:
with open(app_path, mode='rb') as afile:

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2). This path depends on a [user-provided value](3). This path depends on a [user-provided value](4).

Copilot Autofix AI about 2 months ago

To fix the problem, we need to ensure that the app_path is validated and sanitized before being used in file operations. This can be achieved by normalizing the path and ensuring it is within a predefined safe directory.

  1. Normalize the app_path using os.path.normpath to remove any ".." segments.
  2. Check that the normalized path starts with the expected base directory.
  3. If the path is not valid, raise an exception or handle the error appropriately.
Suggested changeset 1
mobsf/StaticAnalyzer/views/common/shared_func.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/mobsf/StaticAnalyzer/views/common/shared_func.py b/mobsf/StaticAnalyzer/views/common/shared_func.py
--- a/mobsf/StaticAnalyzer/views/common/shared_func.py
+++ b/mobsf/StaticAnalyzer/views/common/shared_func.py
@@ -64,2 +64,7 @@
         block_size = 65536
+        # Validate and sanitize app_path
+        base_path = os.path.join(settings.UPLD_DIR, checksum + '/')
+        app_path = os.path.normpath(app_path)
+        if not app_path.startswith(base_path):
+            raise Exception("Invalid app_path")
         with open(app_path, mode='rb') as afile:
@@ -85,2 +90,7 @@
         files = []
+        # Validate and sanitize app_path
+        base_path = os.path.join(settings.UPLD_DIR, checksum + '/')
+        app_path = os.path.normpath(app_path)
+        if not app_path.startswith(base_path):
+            raise Exception("Invalid app_path")
         with zipfile.ZipFile(app_path, 'r') as zipptr:
EOF
@@ -64,2 +64,7 @@
block_size = 65536
# Validate and sanitize app_path
base_path = os.path.join(settings.UPLD_DIR, checksum + '/')
app_path = os.path.normpath(app_path)
if not app_path.startswith(base_path):
raise Exception("Invalid app_path")
with open(app_path, mode='rb') as afile:
@@ -85,2 +90,7 @@
files = []
# Validate and sanitize app_path
base_path = os.path.join(settings.UPLD_DIR, checksum + '/')
app_path = os.path.normpath(app_path)
if not app_path.startswith(base_path):
raise Exception("Invalid app_path")
with zipfile.ZipFile(app_path, 'r') as zipptr:
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
'hash': md5_hash,
})
# Walk through the directory
for file_path in Path(src).rglob('*'):

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2).

Copilot Autofix AI about 2 months ago

To fix the problem, we need to validate the src parameter to ensure it does not contain any malicious input that could lead to path traversal vulnerabilities. The best way to do this is to normalize the path using os.path.normpath and then check that the normalized path starts with a predefined safe root directory. This ensures that the path is contained within a safe directory and does not traverse outside of it.

  1. Normalize the src path using os.path.normpath.
  2. Check that the normalized path starts with a predefined safe root directory.
  3. Raise an exception if the path is not within the safe root directory.
Suggested changeset 1
mobsf/StaticAnalyzer/views/ios/file_analysis.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/mobsf/StaticAnalyzer/views/ios/file_analysis.py b/mobsf/StaticAnalyzer/views/ios/file_analysis.py
--- a/mobsf/StaticAnalyzer/views/ios/file_analysis.py
+++ b/mobsf/StaticAnalyzer/views/ios/file_analysis.py
@@ -6,2 +6,3 @@
 from pathlib import Path
+import os
 
@@ -35,4 +36,14 @@
 
+        # Define a safe root directory
+        safe_root = Path(settings.UPLD_DIR)
+
+        # Normalize the src path
+        normalized_src = Path(os.path.normpath(src))
+
+        # Check if the normalized path is within the safe root directory
+        if not normalized_src.resolve().startswith(safe_root.resolve()):
+            raise Exception("Path traversal attempt detected")
+
         # Walk through the directory
-        for file_path in Path(src).rglob('*'):
+        for file_path in normalized_src.rglob('*'):
             if (file_path.is_file()
@@ -48,3 +59,3 @@
                 # Append file details
-                relative_path = file_path.relative_to(src)
+                relative_path = file_path.relative_to(normalized_src)
                 filez.append(str(relative_path))
EOF
@@ -6,2 +6,3 @@
from pathlib import Path
import os

@@ -35,4 +36,14 @@

# Define a safe root directory
safe_root = Path(settings.UPLD_DIR)

# Normalize the src path
normalized_src = Path(os.path.normpath(src))

# Check if the normalized path is within the safe root directory
if not normalized_src.resolve().startswith(safe_root.resolve()):
raise Exception("Path traversal attempt detected")

# Walk through the directory
for file_path in Path(src).rglob('*'):
for file_path in normalized_src.rglob('*'):
if (file_path.is_file()
@@ -48,3 +59,3 @@
# Append file details
relative_path = file_path.relative_to(src)
relative_path = file_path.relative_to(normalized_src)
filez.append(str(relative_path))
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
mobsf/StaticAnalyzer/views/ios/icon_analysis.py Dismissed Show dismissed Hide dismissed
out = subprocess.run(args, capture_output=True)
except Exception:
# Fails or PNG is not crushed
shutil.copy2(icon_file, outfile.as_posix())

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2). This path depends on a [user-provided value](1). This path depends on a [user-provided value](2).

Copilot Autofix AI about 2 months ago

To fix the problem, we need to validate the paths constructed from user-controlled data before using them to access files. We will normalize the path using os.path.normpath and ensure that the resulting path is within a safe root directory. This will prevent directory traversal attacks and ensure that only intended files are accessed.

  1. Normalize the bin_path using os.path.normpath.
  2. Check that the normalized bin_path starts with the expected root directory.
  3. Apply similar validation to the icon_file path before using it.
Suggested changeset 1
mobsf/StaticAnalyzer/views/ios/icon_analysis.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/mobsf/StaticAnalyzer/views/ios/icon_analysis.py b/mobsf/StaticAnalyzer/views/ios/icon_analysis.py
--- a/mobsf/StaticAnalyzer/views/ios/icon_analysis.py
+++ b/mobsf/StaticAnalyzer/views/ios/icon_analysis.py
@@ -30,3 +30,6 @@
         append_scan_status(md5, msg)
-        bin_path = os.path.join(bin_dir, binary + '.app')
+        bin_path = os.path.normpath(os.path.join(bin_dir, binary + '.app'))
+        if not bin_path.startswith(os.path.normpath(bin_dir)):
+            logger.warning('App binary directory path traversal attempt detected')
+            return
         if not is_dir_exists(bin_path):
@@ -38,3 +41,6 @@
             return
-        icon_file = icons.pop()
+        icon_file = os.path.normpath(icons.pop())
+        if not icon_file.startswith(bin_path):
+            logger.warning('App icon path traversal attempt detected')
+            return
         outfile = Path(settings.DWD_DIR) / f'{md5}-icon.png'
EOF
@@ -30,3 +30,6 @@
append_scan_status(md5, msg)
bin_path = os.path.join(bin_dir, binary + '.app')
bin_path = os.path.normpath(os.path.join(bin_dir, binary + '.app'))
if not bin_path.startswith(os.path.normpath(bin_dir)):
logger.warning('App binary directory path traversal attempt detected')
return
if not is_dir_exists(bin_path):
@@ -38,3 +41,6 @@
return
icon_file = icons.pop()
icon_file = os.path.normpath(icons.pop())
if not icon_file.startswith(bin_path):
logger.warning('App icon path traversal attempt detected')
return
outfile = Path(settings.DWD_DIR) / f'{md5}-icon.png'
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
out = subprocess.run(args, capture_output=True)
except Exception:
# Fails or PNG is not crushed
shutil.copy2(icon_file, outfile.as_posix())

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2). This path depends on a [user-provided value](1). This path depends on a [user-provided value](2).

Copilot Autofix AI about 2 months ago

To fix the problem, we need to ensure that the constructed file path is contained within a safe root folder. We can achieve this by normalizing the path using os.path.normpath and then checking that the normalized path starts with the root folder. This will prevent any path traversal attacks and ensure that the file operations are performed within the intended directory.

  1. Normalize the outfile path using os.path.normpath.
  2. Check that the normalized path starts with the settings.DWD_DIR directory.
  3. If the check fails, raise an exception or handle the error appropriately.
Suggested changeset 1
mobsf/StaticAnalyzer/views/ios/icon_analysis.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/mobsf/StaticAnalyzer/views/ios/icon_analysis.py b/mobsf/StaticAnalyzer/views/ios/icon_analysis.py
--- a/mobsf/StaticAnalyzer/views/ios/icon_analysis.py
+++ b/mobsf/StaticAnalyzer/views/ios/icon_analysis.py
@@ -40,2 +40,5 @@
         outfile = Path(settings.DWD_DIR) / f'{md5}-icon.png'
+        normalized_outfile = os.path.normpath(outfile)
+        if not normalized_outfile.startswith(os.path.normpath(settings.DWD_DIR)):
+            raise Exception("Invalid file path")
         app_dict['icon_path'] = outfile.name
EOF
@@ -40,2 +40,5 @@
outfile = Path(settings.DWD_DIR) / f'{md5}-icon.png'
normalized_outfile = os.path.normpath(outfile)
if not normalized_outfile.startswith(os.path.normpath(settings.DWD_DIR)):
raise Exception("Invalid file path")
app_dict['icon_path'] = outfile.name
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
shutil.copy2(icon_file, outfile.as_posix())
else:
logger.warning('CgbiPngFix not available for %s %s', system, arch)
shutil.copy2(icon_file, outfile.as_posix())

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2). This path depends on a [user-provided value](1). This path depends on a [user-provided value](2).

Copilot Autofix AI about 2 months ago

To fix the problem, we need to ensure that the constructed file paths are validated before being used. We can achieve this by normalizing the paths and ensuring they are within a safe root directory. Specifically, we will:

  1. Normalize the bin_path and icon_file paths using os.path.normpath.
  2. Ensure that the normalized paths start with the expected root directory (bin_dir).
Suggested changeset 1
mobsf/StaticAnalyzer/views/ios/icon_analysis.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/mobsf/StaticAnalyzer/views/ios/icon_analysis.py b/mobsf/StaticAnalyzer/views/ios/icon_analysis.py
--- a/mobsf/StaticAnalyzer/views/ios/icon_analysis.py
+++ b/mobsf/StaticAnalyzer/views/ios/icon_analysis.py
@@ -30,3 +30,6 @@
         append_scan_status(md5, msg)
-        bin_path = os.path.join(bin_dir, binary + '.app')
+        bin_path = os.path.normpath(os.path.join(bin_dir, binary + '.app'))
+        if not bin_path.startswith(os.path.normpath(bin_dir)):
+            logger.warning('App binary directory path traversal detected')
+            return
         if not is_dir_exists(bin_path):
@@ -34,3 +37,3 @@
             return
-        icons = glob.glob(bin_path + '/AppIcon*png')
+        icons = glob.glob(os.path.join(bin_path, 'AppIcon*png'))
         if not icons:
@@ -38,3 +41,6 @@
             return
-        icon_file = icons.pop()
+        icon_file = os.path.normpath(icons.pop())
+        if not icon_file.startswith(bin_path):
+            logger.warning('App icon path traversal detected')
+            return
         outfile = Path(settings.DWD_DIR) / f'{md5}-icon.png'
EOF
@@ -30,3 +30,6 @@
append_scan_status(md5, msg)
bin_path = os.path.join(bin_dir, binary + '.app')
bin_path = os.path.normpath(os.path.join(bin_dir, binary + '.app'))
if not bin_path.startswith(os.path.normpath(bin_dir)):
logger.warning('App binary directory path traversal detected')
return
if not is_dir_exists(bin_path):
@@ -34,3 +37,3 @@
return
icons = glob.glob(bin_path + '/AppIcon*png')
icons = glob.glob(os.path.join(bin_path, 'AppIcon*png'))
if not icons:
@@ -38,3 +41,6 @@
return
icon_file = icons.pop()
icon_file = os.path.normpath(icons.pop())
if not icon_file.startswith(bin_path):
logger.warning('App icon path traversal detected')
return
outfile = Path(settings.DWD_DIR) / f'{md5}-icon.png'
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
shutil.copy2(icon_file, outfile.as_posix())
else:
logger.warning('CgbiPngFix not available for %s %s', system, arch)
shutil.copy2(icon_file, outfile.as_posix())

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2). This path depends on a [user-provided value](1). This path depends on a [user-provided value](2).

Copilot Autofix AI about 2 months ago

To fix the problem, we need to ensure that the constructed file path is safe and does not allow path traversal or access to unintended files. We can achieve this by normalizing the path and verifying that it is contained within a safe root directory.

  1. Normalize the outfile path using os.path.normpath to remove any ".." segments.
  2. Check that the normalized path starts with the root directory (settings.DWD_DIR).
  3. If the path is not within the root directory, raise an exception or handle the error appropriately.
Suggested changeset 1
mobsf/StaticAnalyzer/views/ios/icon_analysis.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/mobsf/StaticAnalyzer/views/ios/icon_analysis.py b/mobsf/StaticAnalyzer/views/ios/icon_analysis.py
--- a/mobsf/StaticAnalyzer/views/ios/icon_analysis.py
+++ b/mobsf/StaticAnalyzer/views/ios/icon_analysis.py
@@ -40,2 +40,5 @@
         outfile = Path(settings.DWD_DIR) / f'{md5}-icon.png'
+        outfile = Path(os.path.normpath(outfile))
+        if not str(outfile).startswith(str(Path(settings.DWD_DIR))):
+            raise Exception("Invalid file path")
         app_dict['icon_path'] = outfile.name
EOF
@@ -40,2 +40,5 @@
outfile = Path(settings.DWD_DIR) / f'{md5}-icon.png'
outfile = Path(os.path.normpath(outfile))
if not str(outfile).startswith(str(Path(settings.DWD_DIR))):
raise Exception("Invalid file path")
app_dict['icon_path'] = outfile.name
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
app_dic['app_path'],
app_dic['app_dir'])
# Identify Payload directory
dirs = app_dic['app_dirp'].glob('**/*')

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression

This path depends on a [user-provided value](1). This path depends on a [user-provided value](2).

Copilot Autofix AI about 2 months ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

@github-advanced-security
Copy link

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

mobsf/DynamicAnalyzer/views/common/shared.py Dismissed Show dismissed Hide dismissed
Copy link
Member Author

@ajinabraham ajinabraham left a comment

Choose a reason for hiding this comment

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

LGTM, CodeQL findings are false positives.

@ajinabraham ajinabraham merged commit b5da756 into master Nov 18, 2024
9 checks passed
@ajinabraham ajinabraham deleted the 4.2.0 branch November 18, 2024 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant