Merge pull request #65 from MobSF/rule_update

iOS rule updates, scan type support, Rule QA, Explicit scan_type support.

.github/workflows/tests.yml

@@ -26,7 +26,7 @@ jobs:
     - name: Install dependencies
       run: |
         python -m pip install --upgrade pip
-        pip install tox semgrep
+        pip install tox semgrep==1.21.0
 
     - name: Lint
       run: |
@@ -42,7 +42,7 @@ jobs:
 
     - name: Semgrep tests
       run: |
-        semgrep --quiet --test --config ./mobsfscan/rules/semgrep/ ./tests/assets/rules/semgrep/
+        SEMGREP_SETTINGS_FILE=/dev/null semgrep --metrics=off --test --config ./mobsfscan/rules/semgrep/ ./tests/assets/rules/semgrep/
 
     - name: Run tests
       run: |

README.md

@@ -31,8 +31,7 @@ Requires Python 3.7+
 
 ```bash
 $ mobsfscan
-usage: mobsfscan [-h] [--json] [--sarif] [--sonarqube] [--html] [-o OUTPUT] [-c CONFIG] [-w] [--no-fail] [-v]
-                 [path ...]
+usage: mobsfscan [-h] [--json] [--sarif] [--sonarqube] [--html] [--type {android,ios,auto}] [-o OUTPUT] [-c CONFIG] [-w] [--no-fail] [-v] [path ...]
 
 positional arguments:
   path                  Path can be file(s) or directories with source code
@@ -43,10 +42,12 @@ optional arguments:
   --sarif               set output format as SARIF 2.1.0
   --sonarqube           set output format compatible with SonarQube
   --html                set output format as HTML
+  --type {android,ios,auto}
+                        optional: force android or ios rules explicitly
   -o OUTPUT, --output OUTPUT
                         output filename to save the result
   -c CONFIG, --config CONFIG
-                        Location to .mobsf config file
+                        location to .mobsf config file
   -w, --exit-warning    non zero exit code on warning
   --no-fail             force zero exit code, takes precedence over --exit-warning
   -v, --version         show mobsfscan version
@@ -60,7 +61,7 @@ $ mobsfscan tests/assets/src/
 - Pattern Match ████████████████████████████████████████████████████████████ 3
 - Semantic Grep ██████ 37
 
-mobsfscan: v0.0.2 | Ajin Abraham | opensecurity.in
+mobsfscan: v0.3.0 | Ajin Abraham | opensecurity.in
 ╒══════════════╤════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════════╕
 │ RULE ID      │ android_webview_ignore_ssl                                                                                                                             │
 ├──────────────┼────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┤

mobsfscan/__init__.py

@@ -6,7 +6,7 @@
 __title__ = 'mobsfscan'
 __authors__ = 'Ajin Abraham'
 __copyright__ = f'Copyright {datetime.now().year} Ajin Abraham, OpenSecurity'
-__version__ = '0.2.0'
+__version__ = '0.3.0'
 __version_info__ = tuple(int(i) for i in __version__.split('.'))
 __all__ = [
     '__title__',

mobsfscan/__main__.py

@@ -47,11 +47,15 @@ def main():
     parser.add_argument('--html',
                         help='set output format as HTML',
                         action='store_true')
+    parser.add_argument('--type',
+                        help='optional: force android or ios rules explicitly',
+                        choices=['android', 'ios', 'auto'],
+                        default='auto')
     parser.add_argument('-o', '--output',
                         help='output filename to save the result',
                         required=False)
     parser.add_argument('-c', '--config',
-                        help='Location to .mobsf config file',
+                        help='location to .mobsf config file',
                         required=False)
     parser.add_argument('-w', '--exit-warning',
                         help='non zero exit code on warning',
@@ -74,6 +78,7 @@ def main():
         scan_results = MobSFScan(
             args.path,
             is_json,
+            args.type,
             args.config,
         ).scan()
         if args.sonarqube:

mobsfscan/mobsfscan.py

@@ -21,7 +21,8 @@
 
 
 class MobSFScan:
-    def __init__(self, paths, json, config=False) -> None:
+    def __init__(self, paths, json, scan_type='auto', config=False) -> None:
+        self.scan_type = scan_type
         self.conf = get_config(paths, config)
         self.options = {
             'match_rules': None,
@@ -48,6 +49,11 @@ def __init__(self, paths, json, config=False) -> None:
 
     def rules_selector(self, suffix):
         """Get rule extensions from suffix."""
+        if self.scan_type == 'android':
+            suffix = '.kt'
+        elif self.scan_type == 'ios':
+            suffix = '.swift'
+        # Default to .kt/.swift best practices if scan_type is specified
         if suffix in ['.java', '.kt']:
             if suffix == '.java':
                 self.best_practices = '.java'
@@ -98,7 +104,7 @@ def scan(self) -> dict:
         result = scanner.scan()
         try:
             # Keep beta for a while
-            if self.xmls:
+            if self.xmls and self.scan_type in ('auto', 'android'):
                 result['xml_checks'] = manifest.scan_manifest(
                     self.xmls)
         except Exception:

mobsfscan/rules/patterns/android/kotlin/kotlin_rules.yaml

@@ -34,7 +34,7 @@
     modified by any application.
   type: RegexAnd
   pattern:
-    - \.loadUrl\(.*getExternalStorageDirectory\(
+    - \.loadUrl\(.{0,48}getExternalStorageDirectory\(
     - webkit\.WebView
   severity: ERROR
   input_case: exact
@@ -171,8 +171,8 @@
 - id: cbc_kotlin_padding_oracle
   message: The App uses the encryption mode CBC with PKCS5/PKCS7 padding. This configuration is vulnerable to padding oracle attacks.
   pattern:
-    - \.getInstance\(.*\/CBC\/PKCS5Padding
-    - \.getInstance\(.*\/CBC\/PKCS7Padding
+    - \.getInstance\(.{0,48}\/CBC\/PKCS5Padding
+    - \.getInstance\(.{0,48}\/CBC\/PKCS7Padding
   type: RegexOr
   severity: ERROR
   input_case: exact
@@ -187,7 +187,7 @@
     scheme is to prevent a number of attacks on RSA that only work when the
     encryption is performed without padding.
   type: Regex
-  pattern: cipher\.getinstance\(\"rsa/.+/nopadding
+  pattern: cipher\.getinstance\(\"rsa/.{1,48}/nopadding
   severity: ERROR
   input_case: lower
   metadata:
@@ -200,7 +200,7 @@
   type: RegexOr
   pattern:
     - MODE_WORLD_WRITABLE
-    - 'openFileOutput\(\s*".+"\s*,\s*2\s*\)'
+    - 'openFileOutput\(\s*".{1,48}"\s*,\s*2\s*\)'
   severity: WARNING
   input_case: exact
   metadata:
@@ -213,7 +213,7 @@
   type: RegexOr
   pattern:
     - MODE_WORLD_READABLE
-    - 'openFileOutput\(\s*".+"\s*,\s*1\s*\)'
+    - 'openFileOutput\(\s*".{1,48}"\s*,\s*1\s*\)'
   severity: WARNING
   input_case: exact
   metadata:
@@ -224,7 +224,7 @@
 - id: android_kotlin_world_read_write
   message: The file is World Readable and Writable. Any App can read/write to the file
   type: Regex
-  pattern: 'openFileOutput\(\s*".+"\s*,\s*3\s*\)'
+  pattern: 'openFileOutput\(\s*".{1,48}"\s*,\s*3\s*\)'
   severity: WARNING
   input_case: exact
   metadata:
@@ -235,8 +235,8 @@
 - id: android_kotlin_weak_hash
   message: Weak Hash algorithm used. The hash algorithm is known to have hash collisions.
   pattern:
-    - \.getInstance\(.*md4
-    - \.getInstance\(.*MD4
+    - \.getInstance\(.{0,48}md4
+    - \.getInstance\(.{0,48}MD4
   type: RegexOr
   input_case: exact
   severity: WARNING
@@ -251,14 +251,14 @@
   type: RegexOr
   input_case: exact
   pattern:
-    - \.getInstance\(.*rc2
-    - \.getInstance\(.*RC2
-    - \.getInstance\(.*rc4
-    - \.getInstance\(.*RC4
-    - \.getInstance\(.*blowfish
-    - \.getInstance\(.*BLOWFISH
-    - Cipher\.getInstance\(.*DES
-    - Cipher\.getInstance\(.*des
+    - \.getInstance\(.{0,48}rc2
+    - \.getInstance\(.{0,48}RC2
+    - \.getInstance\(.{0,48}rc4
+    - \.getInstance\(.{0,48}RC4
+    - \.getInstance\(.{0,48}blowfish
+    - \.getInstance\(.{0,48}BLOWFISH
+    - Cipher\.getInstance\(.{0,48}DES
+    - Cipher\.getInstance\(.{0,48}des
   metadata:
     cwe: cwe-327
     masvs: crypto-4
@@ -268,8 +268,8 @@
   message: MD5 is a weak hash known to have hash collisions.
   type: RegexOr
   pattern:
-      - \.getInstance\(.*MD5
-      - \.getInstance\(.*md5
+      - \.getInstance\(.{0,48}MD5
+      - \.getInstance\(.{0,48}md5
       - DigestUtils\.md5\(
   input_case: exact
   severity: WARNING
@@ -284,10 +284,10 @@
   input_case: exact
   severity: WARNING
   pattern:
-    - \.getInstance\(.*SHA-1
-    - \.getInstance\(.*sha-1
-    - \.getInstance\(.*SHA1
-    - \.getInstance\(.*sha1
+    - \.getInstance\(.{0,48}SHA-1
+    - \.getInstance\(.{0,48}sha-1
+    - \.getInstance\(.{0,48}SHA1
+    - \.getInstance\(.{0,48}sha1
     - DigestUtils\.sha\(
   metadata:
     cwe: cwe-327
@@ -317,7 +317,7 @@
     passwords, keys etc.
   input_case: lower
   pattern: >-
-    (password\s*=\s*[\'|\"].+[\'|\"]\s{0,5})|(pass\s*=\s*[\'|\"].+[\'|\"]\s{0,5})|(username\s*=\s*[\'|\"].+[\'|\"]\s{0,5})|(secret\s*=\s*[\'|\"].+[\'|\"]\s{0,5})|(key\s*=\s*[\'|\"].+[\'|\"]\s{0,5})
+    (password\s*=\s*[\'|\"].{1,100}[\'|\"]\s{0,5})|(pass\s*=\s*[\'|\"].{1,100}[\'|\"]\s{0,5})|(username\s*=\s*[\'|\"].{1,100}[\'|\"]\s{0,5})|(secret\s*=\s*[\'|\"].{1,100}[\'|\"]\s{0,5})|(key\s*=\s*[\'|\"].{1,100}[\'|\"]\s{0,5})
   severity: WARNING
   type: Regex
   metadata:

mobsfscan/rules/patterns/ios/objectivec/objective_c_rules.yaml

@@ -2,7 +2,7 @@
   message: Files may contain hardcoded sensitive information like usernames,
     passwords, keys etc.
   input_case: lower
-  pattern: (password\s*=\s*[\'|\"].+[\'|\"]\s{0,5})|(pass\s*=\s*[\'|\"].+[\'|\"]\s{0,5})|(username\s*=\s*[\'|\"].+[\'|\"]\s{0,5})|(secret\s*=\s*[\'|\"].+[\'|\"]\s{0,5})|(key\s*=\s*[\'|\"].+[\'|\"]\s{0,5})
+  pattern: (password\s*=\s*[\'|\"].{1,100}[\'|\"]\s{0,5})|(pass\s*=\s*[\'|\"].{1,100}[\'|\"]\s{0,5})|(username\s*=\s*[\'|\"].{1,100}[\'|\"]\s{0,5})|(secret\s*=\s*[\'|\"].{1,100}[\'|\"]\s{0,5})|(key\s*=\s*[\'|\"].{1,100}[\'|\"]\s{0,5})
   severity: WARNING
   type: Regex
   metadata:
@@ -28,7 +28,7 @@
     The App may contain banned API(s). These API(s) are insecure and must not be
     used.
   input_case: exact
-  pattern: '\s+(strcpy\(|memcpy\(|strcat\(|strncat\(|strncpy\(|sprintf\(|sprintf\(|gets\()'
+  pattern: '( strcpy\(| memcpy\(| strcat\(| strncat\(| strncpy\(| sprintf\(| sprintf\(| gets\()'
   severity: WARNING
   type: Regex
   metadata:

mobsfscan/rules/patterns/ios/swift/best_practices.yaml

@@ -62,7 +62,7 @@
 - id: ios_custom_keyboard_disabled
   message: This app does not have custom keyboards disabled.
   input_case: exact
-  pattern: extensionPointIdentifier == .*\.keyboard
+  pattern: extensionPointIdentifier == .{0,100}\.keyboard
   severity: INFO
   type: Regex
   metadata:

mobsfscan/rules/patterns/ios/swift/swift_rules.yaml

@@ -2,7 +2,7 @@
   message: Files may contain hardcoded sensitive information like usernames,
     passwords, keys etc.
   input_case: lower
-  pattern: (password\s*=\s*[\'|\"].+[\'|\"]\s{0,5})|(pass\s*=\s*[\'|\"].+[\'|\"]\s{0,5})|(username\s*=\s*[\'|\"].+[\'|\"]\s{0,5})|(secret\s*=\s*[\'|\"].+[\'|\"]\s{0,5})|(key\s*=\s*[\'|\"].+[\'|\"]\s{0,5})
+  pattern: (password\s*=\s*[\'|\"].{1,100}[\'|\"]\s{0,5})|(pass\s*=\s*[\'|\"].{1,100}[\'|\"]\s{0,5})|(username\s*=\s*[\'|\"].{1,100}[\'|\"]\s{0,5})|(secret\s*=\s*[\'|\"].{1,100}[\'|\"]\s{0,5})|(key\s*=\s*[\'|\"].{1,100}[\'|\"]\s{0,5})
   severity: WARNING
   type: Regex
   metadata:
@@ -26,7 +26,7 @@
 - id: ios_log
   message: The App logs information. Sensitive information should never be logged.
   input_case: exact
-  pattern: (print|NSLog|os_log|OSLog|os_signpost)\(.*\)
+  pattern: (print|NSLog|os_log|OSLog|os_signpost)\(
   severity: INFO
   type: Regex
   metadata:
@@ -68,16 +68,68 @@
     owasp-mobile: m5
     reference: https://github.com/MobSF/owasp-mstg/blob/master/Document/0x04g-Testing-Cryptography.md#identifying-insecure-andor-deprecated-cryptographic-algorithms-mstg-crypto-4
 - id: ios_biometric_bool
-  message: Biometric authentication should be based on Keychain, not based on bool.
+  message: Biometric authentication should be hardware and keychain backed, local authentication returns a boolean that can be bypassed by runtime instrumentation tools like Frida.
   input_case: exact
-  pattern: \.evaluatePolicy\(\w*.deviceOwnerAuthentication
+  pattern:
+    - \.evaluatePolicy\(
+    - \.deviceOwnerAuthentication
+    - LAContext\(
   severity: WARNING
-  type: Regex
+  type: RegexAnd
   metadata:
-    cwe: cwe-919
+    cwe: cwe-303
     masvs: auth-8
     owasp-mobile: m1
     reference: https://github.com/MobSF/owasp-mstg/blob/master/Document/0x06f-Testing-Local-Authentication.md#local-authentication-framework
+- id: ios_biometric_acl
+  message: Weak biometric ACL flag is associated with a key stored in Keychain. With `.biometryAny/.userPresence/.touchIDAny` flag, an attacker with the ability to add a biometry to the device can authenticate as the user. Use `.biometryCurrentSet/.touchIDCurrentSet` instead.
+  input_case: exact
+  pattern:
+    - \.biometryAny|\.userPresence|\.touchIDAny
+    - SecAccessControlCreateWithFlags\(
+  severity: ERROR
+  type: RegexAnd
+  metadata:
+    cwe: cwe-305
+    masvs: auth-8
+    owasp-mobile: m1
+    reference: https://github.com/MobSF/owasp-mstg/blob/master/Document/0x06f-Testing-Local-Authentication.md#local-authentication-framework
+- id: ios_keychain_weak_acl_device_passcode
+  message: A key stored in the Keychain is not making use of stronger biometric backed ACL. Use `.biometryCurrentSet` instead.
+  input_case: exact
+  pattern:
+    - \.devicePasscode
+    - SecAccessControlCreateWithFlags\(
+  severity: WARNING
+  type: RegexAnd
+  metadata:
+    cwe: cwe-305
+    masvs: auth-8
+    owasp-mobile: m1
+    reference: https://github.com/MobSF/owasp-mstg/blob/master/Document/0x06f-Testing-Local-Authentication.md
+- id: ios_keychain_weak_accessibility_value
+  message: A key stored in the Keychain is using a weak accessibility value. Use stronger ACLs like `kSecAttrAccessibleWhenPasscodeSetThisDeviceOnly/kSecAttrAccessibleWhenUnlocked/kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly`.
+  input_case: exact
+  pattern: kSecAttrAccessibleAlways|kSecAttrAccessibleAfterFirstUnlock
+  severity: WARNING
+  type: Regex
+  metadata:
+    cwe: cwe-305
+    masvs: auth-8
+    owasp-mobile: m1
+    reference: https://github.com/MobSF/owasp-mstg/blob/master/Document/0x06f-Testing-Local-Authentication.md
+- id: ios_insecure_random_no_generator
+  message: The App uses an insecure Random Number Generator.
+  input_case: exact
+  pattern: ((Int|Bool|Float|Double)\.random\()|arc4random.{0,10}\(|SystemRandomNumberGenerator\(|rand\(| random\(
+  severity: WARNING
+  type: Regex
+  metadata:
+    input_case: exact
+    cwe: cwe-330
+    owasp-mobile: m5
+    masvs: crypto-6
+    reference: https://github.com/MobSF/owasp-mstg/blob/master/Document/0x04g-Testing-Cryptography.md#weak-random-number-generators
 - id: ios_file_no_special
   message: The file has no special protections associated with it.
   input_case: exact

mobsfscan/rules/semgrep/android/secrets.yaml

@@ -10,7 +10,7 @@ rules:
               $M($X, "...", ...);
       - metavariable-regex:
           metavariable: "$X"
-          regex: "(?i:.*pass.*)"
+          regex: "(?i:^.{0,100}pass.{0,100})"
     message: >-
       A hardcoded password in plain text is identified.
     languages:
@@ -32,7 +32,7 @@ rules:
               $M($X, "...", ...);
       - metavariable-regex:
           metavariable: "$X"
-          regex: "(?i:.*user.*)"
+          regex: "(?i:^.{0,100}user.{0,100})"
     message: >-
       A hardcoded username in plain text is identified.
     languages:
@@ -54,7 +54,7 @@ rules:
               $M($X, "...", ...);
       - metavariable-regex:
           metavariable: "$X"
-          regex: "(?i:.*key.*)"
+          regex: "(?i:^.{0,100}key.{0,100})"
     message: >-
       A hardcoded Key is identified.
     languages:
@@ -76,7 +76,7 @@ rules:
               $M($X, "...", ...);
       - metavariable-regex:
           metavariable: "$X"
-          regex: "(?i:.*secret)"
+          regex: "(?i:^.{0,100}secret.{0,100})"
     message: >-
       A hardcoded secret is identified.
     languages: