Skip to content

Commit

Permalink
[HOTFIX][EFR] Priority Bug Fixes (MobSF#2275)
Browse files Browse the repository at this point in the history
* P1.1 AAR Permissions not properly listed 
* P1.2 Local variable table not listed in proper section
* P1.3 static library strings are not listed
* P1.5 Stripping of dynamic and static libraries are not correctly reported
* Dependency bump
* MobSF version bump
  • Loading branch information
ajinabraham authored Oct 7, 2023
1 parent abb4765 commit 98296f5
Show file tree
Hide file tree
Showing 13 changed files with 766 additions and 622 deletions.
2 changes: 1 addition & 1 deletion mobsf/MobSF/init.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

logger = logging.getLogger(__name__)

VERSION = '3.7.8'
VERSION = '3.7.9'
BANNER = """
__ __ _ ____ _____ _____ _____
| \/ | ___ | |__/ ___|| ___|_ _|___ /|___ |
Expand Down
33 changes: 27 additions & 6 deletions mobsf/StaticAnalyzer/views/android/jar_aar.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'])
Expand All @@ -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(
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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
2 changes: 1 addition & 1 deletion mobsf/StaticAnalyzer/views/android/manifest_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions mobsf/StaticAnalyzer/views/android/manifest_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
12 changes: 10 additions & 2 deletions mobsf/StaticAnalyzer/views/common/binary/elf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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 = []
Expand Down
42 changes: 31 additions & 11 deletions mobsf/StaticAnalyzer/views/common/binary/macho.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)

Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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, '
Expand Down Expand Up @@ -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 = []
Expand Down
36 changes: 36 additions & 0 deletions mobsf/StaticAnalyzer/views/common/binary/strings.py
Original file line number Diff line number Diff line change
@@ -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')
11 changes: 0 additions & 11 deletions mobsf/StaticAnalyzer/views/common/shared_func.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion mobsf/StaticAnalyzer/views/ios/binary_analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
18 changes: 0 additions & 18 deletions mobsf/StaticAnalyzer/views/ios/strings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)

Expand Down Expand Up @@ -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
Expand Down
12 changes: 0 additions & 12 deletions mobsf/templates/pdf/android_report.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand Down
12 changes: 0 additions & 12 deletions mobsf/templates/static_analysis/android_binary_analysis.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Loading

0 comments on commit 98296f5

Please sign in to comment.