Merge pull request #7 from MobSF/master

TRIVAL: Priority Bug Fixes

mobsf/MobSF/init.py

@@ -10,7 +10,7 @@
 
 logger = logging.getLogger(__name__)
 
-VERSION = '3.7.8'
+VERSION = '3.7.9'
 BANNER = """
   __  __       _    ____  _____       _____ _____ 
  |  \/  | ___ | |__/ ___||  ___|_   _|___ /|___  |

mobsf/StaticAnalyzer/views/android/jar_aar.py

@@ -143,7 +143,6 @@ def common_analysis(request, app_dic, rescan, api, analysis_type):
             }
         app_dic['real_name'] = ''
         elf_dict = library_analysis(app_dic['app_dir'], 'elf')
-        apkid_results = obfuscated_check(app_dic['app_dir'])
         tracker = Trackers.Trackers(
             app_dic['app_dir'],
             app_dic['tools_dir'])
@@ -158,7 +157,7 @@ def common_analysis(request, app_dic, rescan, api, analysis_type):
             app_dic['app_dir'],
             'apk',
             app_dic['manifest_file'])
-
+        obfuscated_check(app_dic['app_dir'], code_an_dic)
         quark_results = []
         # Get the strings and metadata
         get_strings_metadata(
@@ -186,7 +185,7 @@ def common_analysis(request, app_dic, rescan, api, analysis_type):
             code_an_dic,
             cert_dic,
             elf_dict['elf_analysis'],
-            apkid_results,
+            {},
             quark_results,
             tracker_res,
             rescan,
@@ -216,9 +215,20 @@ def aar_analysis(request, app_dic, rescan, api):
     return common_analysis(request, app_dic, rescan, api, 'aar')
 
 
-def obfuscated_check(src):
+def obfuscated_check(src, code_an_dic):
     """Check if JAR/AAR is obfuscated."""
     logger.info('Checking for Obfuscation')
+    metadata = {
+        'cvss': 0,
+        'cwe': '',
+        'owasp-mobile': '',
+        'masvs': '',
+        'ref': '',
+        'description': (
+            'The binary might not be obfuscated.'
+            ' LocalVariableTable is present in class file.'),
+        'severity': 'info',
+    }
     try:
         app_dir = Path(src)
         # Extract all jar files
@@ -235,7 +245,18 @@ def obfuscated_check(src):
             cls_dat = i.read_text(
                 encoding='utf-8', errors='ignore')
             if 'LocalVariableTable' in cls_dat:
-                return {'obfuscated': False}
+                code_an_dic['findings']['aar_class_obfuscation'] = {
+                    'files': {i.name: '1,1'},
+                    'metadata': metadata,
+                }
+                return
     except Exception:
         logger.exception('Obfuscation Check')
-    return {'obfuscated': True}
+    metadata['description'] = (
+        'The binary might be obfuscated.'
+        ' LocalVariableTable is absent in class file.')
+    code_an_dic['findings']['aar_class_obfuscation'] = {
+        'files': {},
+        'metadata': metadata,
+    }
+    return

mobsf/StaticAnalyzer/views/android/manifest_analysis.py

@@ -431,7 +431,7 @@ def manifest_analysis(mfxml, ns, man_data_dic, src_type, app_dir):
                                     # Esteve 24.07.2016 - end
                                     # Esteve 29.07.2016 - begin The component is not explicitly exported (android:exported is not 'true'). It is not implicitly exported either (it does not
                                     # make use of an intent filter). Despite that, it could still be exported by default, if it is a content provider and the android:targetSdkVersion
-                                    # is older than 17 (Jelly Bean, Android versionn 4.2). This is true regardless of the system's API level.
+                                    # is older than 17 (Jelly Bean, Android version 4.2). This is true regardless of the system's API level.
                                     # Finally, it must also be taken into account that, if the minSdkVersion is greater or equal than 17, this check is unnecessary, because the
                                     # app will not be run on a system where the
                                     # system's API level is below 17.

mobsf/StaticAnalyzer/views/android/manifest_utils.py

@@ -136,6 +136,8 @@ def get_manifest(app_path, app_dir, tools_dir, typ):
         logger.info('Parsing AndroidManifest.xml')
         xml_str = mfile.read_text('utf-8', 'ignore')
         ns = get_xml_namespace(xml_str)
+        if ns and ns == 'xmlns':
+            ns = 'android'
         if ns and ns != 'android':
             logger.warning('Non standard XML namespace: %s', ns)
         try:

mobsf/StaticAnalyzer/views/common/binary/elf.py

@@ -2,6 +2,10 @@
 # coding=utf-8
 import lief
 
+from mobsf.StaticAnalyzer.views.common.binary.strings import (
+    strings_on_binary,
+)
+
 
 class ELFChecksec:
     def __init__(self, elf_file, so_rel):
@@ -200,10 +204,14 @@ def fortify(self):
         return fortified_funcs
 
     def strings(self):
+        elf_strings = None
         try:
-            return self.elf.strings
+            elf_strings = self.elf.strings
         except Exception:
-            return []
+            elf_strings = None
+        if not elf_strings:
+            elf_strings = strings_on_binary(self.elf_path)
+        return elf_strings
 
     def get_symbols(self):
         symbols = []

mobsf/StaticAnalyzer/views/common/binary/macho.py

@@ -2,7 +2,7 @@
 # coding=utf-8
 import lief
 
-from mobsf.StaticAnalyzer.views.ios.strings import (
+from mobsf.StaticAnalyzer.views.common.binary.strings import (
     strings_on_binary,
 )
 
@@ -91,7 +91,7 @@ def checksec(self):
         elif is_stripped:
             severity = 'warning'
             desc = (
-                'This binary has symbols stripped. We cannot identify '
+                'This binary has debug symbols stripped. We cannot identify '
                 'whether stack canary is enabled or not.')
         else:
             severity = 'high'
@@ -123,7 +123,7 @@ def checksec(self):
         elif is_stripped:
             severity = 'warning'
             desc = (
-                'This binary has symbols stripped. We cannot identify '
+                'This binary has debug symbols stripped. We cannot identify '
                 'whether ARC is enabled or not.')
         else:
             severity = 'high'
@@ -184,11 +184,11 @@ def checksec(self):
         }
         if is_stripped:
             severity = 'info'
-            desc = 'Symbols are stripped'
+            desc = 'Debug Symbols are stripped'
         else:
             severity = 'warning'
             desc = (
-                'Symbols are available. To strip '
+                'Debug Symbols are available. To strip '
                 'debugging symbols, set Strip Debug '
                 'Symbols During Copy to YES, '
                 'Deployment Postprocessing to YES, '
@@ -240,14 +240,34 @@ def is_encrypted(self):
             return False
 
     def is_symbols_stripped(self):
-        filter_symbols = ['radr://5614542', '__mh_execute_header']
+        # Based on issues/1917#issuecomment-1238078359
+        # and issues/2233#issue-1846914047
+        stripped_sym = 'radr://5614542'
+        # radr://5614542 symbol is added back for
+        # debug symbols stripped binaries
         for i in self.macho.symbols:
-            strip_bool = i.type & 0xe0
-            strip_type = i.type in [0x0e, 0x1e, 0x0f]
-            sym = i.name.lower().strip()
-            if (strip_bool > 0 or strip_type) and sym not in filter_symbols:
+            if i.name.lower().strip() in ('__mh_execute_header', stripped_sym):
+                # __mh_execute_header is present in both
+                # stripped and unstripped binaries
+                # also ignore radr://5614542
+                continue
+            if (i.type & 0xe0) > 0 or i.type in (0x0e, 0x1e):
+                # N_STAB set or 14, 30
+
+                # N_STAB	0xe0  /* if any of these bits set,
+                # a symbolic debugging entry */ -> 224
+                # From https://opensource.apple.com/source/xnu/xnu-201/
+                # EXTERNAL_HEADERS/mach-o/nlist.h
+                # Only symbolic debugging entries have
+                # some of the N_STAB bits set and if any
+                # of these bits are set then it is a
+                # symbolic debugging entry (a stab).
+
+                # Identified a debugging symbol
                 return False
-        return True
+        if stripped_sym in self.get_symbols():
+            return True
+        return False
 
     def get_libraries(self):
         libs = []

mobsf/StaticAnalyzer/views/common/binary/strings.py

@@ -0,0 +1,36 @@
+"""Common String Extraction Module."""
+import logging
+import shutil
+import subprocess
+
+from mobsf.StaticAnalyzer.tools.strings import (
+    strings_util,
+)
+
+
+logger = logging.getLogger(__name__)
+
+
+def get_os_strings(filename):
+    try:
+        strings_bin = shutil.which('strings')
+        if not strings_bin:
+            return None
+        strings = subprocess.check_output([strings_bin, filename])
+        return strings.decode('utf-8', 'ignore').splitlines()
+    except Exception:
+        return None
+
+
+def strings_on_binary(bin_path):
+    """Extract strings from binary."""
+    try:
+        strings = get_os_strings(bin_path)
+        if strings:
+            return list(set(strings))
+        if isinstance(strings, list):
+            return []
+        # Only run if OS strings is not present
+        return list(set(strings_util(bin_path)))
+    except Exception:
+        logger.exception('Extracting strings from binary')

mobsf/StaticAnalyzer/views/common/shared_func.py

@@ -361,17 +361,6 @@ def strings_and_entropies(src, exts):
     return data
 
 
-def get_os_strings(filename):
-    try:
-        strings_bin = shutil.which('strings')
-        if not strings_bin:
-            return None
-        strings = subprocess.check_output([strings_bin, filename])
-        return strings.decode('utf-8', 'ignore').splitlines()
-    except Exception:
-        return None
-
-
 def get_symbols(symbols):
     all_symbols = []
     for i in symbols:

mobsf/StaticAnalyzer/views/ios/binary_analysis.py

@@ -15,7 +15,7 @@
 from mobsf.StaticAnalyzer.views.common.binary.lib_analysis import (
     MachOChecksec,
 )
-from mobsf.StaticAnalyzer.views.ios.strings import (
+from mobsf.StaticAnalyzer.views.common.binary.strings import (
     strings_on_binary,
 )
 from mobsf.StaticAnalyzer.views.ios.binary_rule_matcher import (

mobsf/StaticAnalyzer/views/ios/strings.py

@@ -3,14 +3,10 @@
 import io
 import logging
 
-from mobsf.StaticAnalyzer.tools.strings import (
-    strings_util,
-)
 from mobsf.StaticAnalyzer.views.common.entropy import (
     get_entropies,
 )
 from mobsf.StaticAnalyzer.views.common.shared_func import (
-    get_os_strings,
     url_n_email_extract,
 )
 
@@ -60,20 +56,6 @@ def extract_urls_n_email(src, all_files, strings):
     }
 
 
-def strings_on_binary(bin_path):
-    """Extract strings from binary."""
-    try:
-        strings = get_os_strings(bin_path)
-        if strings:
-            return list(set(strings))
-        if isinstance(strings, list):
-            return []
-        # Only run if OS strings is not present
-        return list(set(strings_util(bin_path)))
-    except Exception:
-        logger.exception('Extracting strings from binary')
-
-
 def get_strings_metadata(app_dict, bin_dict, all_files, dy_list):
     """Merge strings metadata."""
     # app_dict has secrets from plist secret analysis

mobsf/templates/pdf/android_report.html

@@ -187,18 +187,6 @@ <h5>Android Version Name:</h5> {{ version_name }}</br>
       <h5>Android Version Code:</h5> {{ version_code }}</br>
 {% endif %}
 
-{% if app_type in 'jar,aar' %}
-<p>
-{% for _, value in apkid.items %}
-   {% if value %}
-     The binary might be obfuscated. LocalVariableTable is absent in class file.
-   {% else %}
-     The binary might not be obfuscated. LocalVariableTable is present in class file.
-   {% endif %}
-{% endfor %}
-</p>
-{% endif %}
-
 {% if app_type not in 'jar,aar,so' %}
       <h2><i class="fas fa-th-large"></i> APP COMPONENTS </h2>
 

mobsf/templates/static_analysis/android_binary_analysis.html

@@ -697,18 +697,6 @@ <h3>{{ providers | length }}</h3>
                     {% endif %}
                     <a href="{% url 'generate_downloads' %}?hash={{ md5 }}&amp;file_type={{ app_type }}" class="btn btn-warning"> <i class="fa fa-download"></i> Download {{ app_type | upper}}</a>
 
-                    {% if app_type in 'jar,aar' %}
-                    <p>
-                    {% for _, value in apkid.items %}
-                       {% if value %}
-                         The binary might be obfuscated. LocalVariableTable is absent in class file.
-                       {% else %}
-                         The binary might not be obfuscated. LocalVariableTable is present in class file.
-                       {% endif %}
-                    {% endfor %}
-                    </p>
-                    {% endif %}
-
                   </p>
                 </div>
               </div>