Harden path traversal against Windows root-relative escapes (#2673)

* Harden path traversal checks for Windows root-relative escapes. Reject POSIX/Windows absolute paths in is_path_traversal and require post-join containment at icon, network-config, and archive extraction sinks. Co-authored-by: Cursor <[email protected]> * Restrict network security config names to a basename. CodeQL flagged the manifest-derived join; reduce the resource name first so path construction cannot escape xml_dir. Co-authored-by: Cursor <[email protected]> * Use os.path.basename for CodeQL-recognized path sanitization. Path.name did not clear py/path-injection taint; basename plus existing containment checks keep the network-config read inside xml_dir. Co-authored-by: Cursor <[email protected]> * Drop CodeQL-appeasement basename sanitization from network config reads. is_safe_path after join already rejects traversal, absolute, and Windows root-relative names; CodeQL py/path-injection is a false positive. Co-authored-by: Cursor <[email protected]> --------- Co-authored-by: Cursor <[email protected]>
2026-10-11 22:50:09 +00:00 · 2026-09-20 16:30:53 -07:00
parent 18bb89b672
commit 388f26ad5e
8 changed files with 113 additions and 58 deletions
@@ -10,6 +10,7 @@ Please report all security issues [here](https://github.com/MobSF/Mobile-Securit

 | Vulnerability | Affected Versions |
 | ------- | ------------------ |
+| [Windows-only path traversal via root-relative paths bypasses is_path_traversal()](https://github.com/MobSF/Mobile-Security-Framework-MobSF/security/advisories/GHSA-c8g7-42qj-frj3) | `<=4.5.3` |
 | [SSRF port restriction bypass in assetlinks_check](https://github.com/MobSF/Mobile-Security-Framework-MobSF/security/advisories/GHSA-95px-34x5-p37h) | `<=4.5.0` |
 | [Zip Bomb Denial of Service via Per-File Size Limit Bypass in ZIP/APK](https://github.com/MobSF/Mobile-Security-Framework-MobSF/security/advisories/GHSA-x768-8642-mmq9) | `<=4.5.0` |
 | [Regression: CSRF checks not enforced after Django migration](https://github.com/MobSF/Mobile-Security-Framework-MobSF/security/advisories/GHSA-3p54-567p-2wpr) | `<=4.5.0` |
@@ -15,7 +15,11 @@ from mobsf.MalwareAnalyzer.views.MalwareDomainCheck import (
    MalwareDomainCheck,
 )
 from mobsf.MobSF.exceptions import PathTraversalError
-from mobsf.MobSF.security import clean_filename, is_pipe_or_link
+from mobsf.MobSF.security import (
+    clean_filename,
+    is_pipe_or_link,
+    is_safe_path,
+)
 from mobsf.MobSF.utils import (
    EMAIL_REGEX,
    URL_REGEX,
@@ -108,8 +112,8 @@ def untar_files(tar_loc, untar_dir):
                        continue
                    member_path = os.path.realpath(
                        os.path.join(safe_root, member.name))
-                    if not (member_path.startswith(safe_root + os.sep)
-                            or member_path == safe_root):
+                    if not is_safe_path(
+                            safe_root, member_path, member.name):
                        raise PathTraversalError(
                            'Attempted Path Traversal in Tar File')
                    safe_members.append(member)
@@ -1,25 +1,29 @@
 """Runtime Executable Tampering Detection."""
-import subprocess
 import functools
+import ipaddress
 import logging
+import ntpath
+import os
+import posixpath
 import re
 import socket
-import ipaddress
-import sys
-from contextlib import contextmanager
-from shutil import which
-from pathlib import Path
-from platform import system
-import os
 import stat
 import string
+import subprocess
+import sys
 import unicodedata
-from urllib.parse import unquote, urljoin, urlparse, urlunsplit
 from concurrent.futures import ThreadPoolExecutor
+from contextlib import contextmanager
+from pathlib import Path
+from platform import system
+from shutil import which
+from urllib.parse import unquote, urljoin, urlparse, urlunsplit

 import requests
 from requests.adapters import HTTPAdapter

+from django.conf import settings
+
 from mobsf.MobSF.utils import (
    find_aapt,
    find_java_binary,
@@ -28,9 +32,6 @@ from mobsf.MobSF.utils import (
    sha256,
 )

-from django.conf import settings
-
-
 logger = logging.getLogger(__name__)
 # Non executable files at host level
 _SKIP = [
@@ -596,39 +597,41 @@ def sanitize_svg(svg_content):


 def is_path_traversal(user_input):
-    """Check for path traversal."""
+    """Check for path traversal with POSIX and Windows path semantics."""
    if not user_input:
        return False

-    # Disallow absolute paths and windows paths and backslashes
-    if os.path.isabs(user_input) or user_input.startswith(('\\', '//')):
-        logger.error('Path traversal attack detected with absolute path')
+    try:
+        path = os.fspath(user_input)
+        if isinstance(path, bytes):
+            path = path.decode('utf-8')
+        if not isinstance(path, str):
+            raise TypeError
+    except (TypeError, UnicodeError):
+        logger.error('Path traversal attack detected with invalid path')
        return True

-    # Normalize and decode URL-encoded characters
+    paths = [path]
    try:
-        # Handle URL decoding (e.g., %2e -> .)
-        decoded = unquote(user_input)
-        # Handle double URL decoding (e.g., %252e -> %2e -> .)
-        double_decoded = unquote(decoded)
+        # Check single and double URL decoding (for example, %252e -> %2e -> .).
+        for _ in range(2):
+            paths.append(unquote(paths[-1]))
    except Exception:
        logger.error('Path traversal attack detected with invalid URL encoding')
        return True

-    # Check for path traversal in both original and decoded versions
-    dangerous_patterns = ['..', '../', '..\\', '..\\\\']
-
-    if any(pattern in user_input for pattern in dangerous_patterns):
-        logger.error('Path traversal attack detected with invalid path')
-        return True
-
-    if any(pattern in decoded for pattern in dangerous_patterns):
-        logger.error('Path traversal attack detected with invalid path')
-        return True
-
-    if any(pattern in double_decoded for pattern in dangerous_patterns):
-        logger.error('Path traversal attack detected with invalid path')
-        return True
+    for candidate in paths:
+        drive, _ = ntpath.splitdrive(candidate)
+        if ('\x00' in candidate
+                or candidate.startswith(('/', '\\'))
+                or posixpath.isabs(candidate)
+                or ntpath.isabs(candidate)
+                or drive):
+            logger.error('Path traversal attack detected with absolute path')
+            return True
+        if '..' in candidate:
+            logger.error('Path traversal attack detected with invalid path')
+            return True
    return False


@@ -636,9 +639,14 @@ def is_safe_path(safe_root, check_path, raw_file):
    """Detect Path Traversal."""
    if is_path_traversal(raw_file):
        return False
-    safe_root = os.path.realpath(os.path.normpath(safe_root))
-    check_path = os.path.realpath(os.path.normpath(check_path))
-    return check_path.startswith(safe_root + os.sep) or check_path == safe_root
+    try:
+        safe_root = os.path.normcase(
+            os.path.realpath(os.path.normpath(safe_root)))
+        check_path = os.path.normcase(
+            os.path.realpath(os.path.normpath(check_path)))
+        return os.path.commonpath((safe_root, check_path)) == safe_root
+    except (TypeError, ValueError):
+        return False


 def clean_filename(filename, replace=' '):
@@ -158,8 +158,11 @@ def install_jadx(mobsf_home, version='1.5.0'):
                for member in zip_ref.namelist():
                    member_path = os.path.realpath(
                        os.path.join(abs_extract_dir, member))
-                    if not (member_path.startswith(abs_extract_dir + os.sep)
-                            or member_path == abs_extract_dir):
+                    common_path = os.path.commonpath((
+                        os.path.normcase(abs_extract_dir),
+                        os.path.normcase(member_path),
+                    ))
+                    if common_path != os.path.normcase(abs_extract_dir):
                        raise PathTraversalError('Attempted Path Traversal in Zip File')
                    zip_ref.extract(member, extract_dir)

@@ -98,8 +98,11 @@ def extract_all_files_from_central_directory(apk_file, central_directory_entries
            # Construct the output file path
            output_path = os.path.realpath(os.path.join(output_dir, filename))
            abs_output_dir = os.path.realpath(output_dir)
-            if not (output_path.startswith(abs_output_dir + os.sep)
-                    or output_path == abs_output_dir):
+            common_path = os.path.commonpath((
+                os.path.normcase(abs_output_dir),
+                os.path.normcase(output_path),
+            ))
+            if common_path != os.path.normcase(abs_output_dir):
                raise PathTraversalError('Attempted Path Traversal in APK Zip Entry')
            # Create directories if necessary
            os.makedirs(os.path.dirname(output_path), exist_ok=True)
@@ -63,6 +63,17 @@ def _search_folder(src, file_pattern):
    return matches


+def _is_safe_icon_path(root, icon_file):
+    """Verify a resolved icon path remains inside its application root."""
+    try:
+        resolved_root = Path(root).resolve()
+        resolved_icon = Path(icon_file).resolve()
+        relative_icon = resolved_icon.relative_to(resolved_root)
+    except (OSError, ValueError):
+        return False
+    return is_safe_path(resolved_root, resolved_icon, relative_icon)
+
+
 def guess_icon_path(res_dir):
    icon_folders = [
        'mipmap-hdpi',
@@ -98,7 +109,9 @@ def get_icon_from_src(app_dic, icon_from_mfst):
        app_dic['md5'],
        res_path,
        icon_from_mfst)
-    if icon_file and Path(icon_file).exists():
+    if (icon_file
+            and Path(icon_file).exists()
+            and _is_safe_icon_path(res_path, icon_file)):
        dwd = Path(settings.DWD_DIR)
        out = dwd / (app_dic['md5'] + '-icon' + Path(icon_file).suffix)
        copy2(icon_file, out)
@@ -188,6 +201,11 @@ def get_icon_apk_res(app_dic):
        if icon_name and any(
                icon_name.startswith(x) for x in RESERVED_FILE_NAMES):
            icon_name = str(Path('_conflict_') / icon_name)
+        if (icon_name
+                and not is_safe_path(
+                    app_dir, app_dir / icon_name, icon_name)):
+            logger.warning('Icon path escapes application directory')
+            icon_name = None

        # androguard/aapt2 cannot find icon file, fallback to res or apktool.
        if not res_path.exists() and apktool_res_path.exists() and not icon_name:
@@ -264,9 +282,11 @@ def get_icon_apk(app_dic):
            # Copy PNG/SVG to Downloads
            icon = app_dic['md5'] + '-icon' + src.suffix.lower()
            out = Path(settings.DWD_DIR) / icon
-            if src and src.exists() and src.is_file():
+            if (src.exists()
+                    and src.is_file()
+                    and _is_safe_icon_path(app_dic['app_dir'], src)):
                copy2(src.as_posix(), out.as_posix())
-            app_dic['icon_path'] = out.name
+                app_dic['icon_path'] = out.name
    except Exception:
        logger.exception('Failed to get icon from APK')

@@ -298,6 +318,8 @@ def get_icon_svg_from_xml(app_dir, icon_xml_file):
    """
    try:
        icon_xml = app_dir / 'apktool_out' / icon_xml_file
+        if not is_safe_path(app_dir, icon_xml, icon_xml_file):
+            return None
        parsed = parseString(icon_xml.read_text('utf8', 'ignore'))
        foreground = parsed.getElementsByTagName('foreground')
        background = parsed.getElementsByTagName('background')
@@ -345,6 +367,8 @@ def convert_axml_to_xml(app_dir, icon_file):
    try:
        logger.info('Converting icon axml to xml')
        icon_bin_xml = app_dir / icon_file
+        if not is_safe_path(app_dir, icon_bin_xml, icon_file):
+            return None
        out_xml = app_dir / icon_file
        aobj = axml.AXMLPrinter(
            icon_bin_xml.read_bytes()).get_xml_obj()
@@ -370,6 +394,8 @@ def convert_vector_to_svg(app_dir, tools_dir, icon_name, apktool_res):
        # When xml is android vector
        values = app_dir / 'res' / 'values'
        direct = app_dir / icon_name
+        if not is_safe_path(app_dir, direct, icon_name):
+            return
        cwd = direct.parent.as_posix()

        # When xml is referencing to vector use apktool_res
@@ -5,7 +5,7 @@ from pathlib import Path

 from defusedxml.minidom import parseString

-from mobsf.MobSF.security import is_path_traversal
+from mobsf.MobSF.security import is_safe_path
 from mobsf.MobSF.utils import (
    append_scan_status,
 )
@@ -30,13 +30,13 @@ def read_netsec_config(checksum, app_dir, config, src_type):
        else:
            # APK
            xml_dir = base / 'apktool_out' / 'res' / 'xml'
-        if not is_path_traversal(config):
-            netsec_file = xml_dir / f'{config}.xml'
-            if netsec_file.exists():
-                desc = f'{msg} from {config}.xml'
-                logger.info(desc)
-                append_scan_status(checksum, desc)
-                return netsec_file.read_text('utf8', 'ignore')
+        netsec_file = xml_dir / f'{config}.xml'
+        if (is_safe_path(xml_dir, netsec_file, config)
+                and netsec_file.exists()):
+            desc = f'{msg} from {config}.xml'
+            logger.info(desc)
+            append_scan_status(checksum, desc)
+            return netsec_file.read_text('utf8', 'ignore')
        # Couldn't find the file defined in manifest
        xmls = Path(xml_dir).glob('*.xml')
        for xml in xmls:
@@ -145,7 +145,10 @@ def unzip(checksum, app_path, ext_path):
                    ext_path = str(Path(ext_path) / '_conflict_')

                # Handle Zip Slip
-                if is_path_traversal(file_path):
+                destination = Path(ext_path) / file_path
+                if (is_path_traversal(file_path)
+                        or not is_safe_path(
+                            ext_path, destination, file_path)):
                    msg = ('Zip slip detected. skipped extracting'
                           f' {sanitize_for_logging(file_path)}')
                    logger.error(msg)
@@ -283,7 +286,9 @@ def ar_os(src, dst):
            [shutil.which('ar'), 't', src],
            stderr=subprocess.STDOUT)
        for raw_file in files.decode('utf-8').split('\n'):
-            if is_path_traversal(raw_file):
+            out = Path(dst) / raw_file
+            if (is_path_traversal(raw_file)
+                    or not is_safe_path(dst, out, raw_file)):
                msg = f'AR slip detected in AR file {src} at {raw_file}'
                logger.error(msg)
                raise ValueError(msg)
@@ -316,6 +321,11 @@ def ar_extract(checksum, src, dst):
                append_scan_status(checksum, msg)
                continue
            out = Path(dst) / filtered
+            if not is_safe_path(dst, out, filtered):
+                msg = f'AR slip detected. skipped extracting {filtered}'
+                logger.warning(msg)
+                append_scan_status(checksum, msg)
+                continue
            out.write_bytes(val.read())
    except Exception:
        # Possibly dealing with Fat binary, needs Mac host