Merge branch 'scan_more_binary' into 'master'

Support more file types in get_embedded_classes

Closes #999

See merge request fdroid/fdroidserver!1123

.gitlab-ci.yml

@@ -265,6 +265,7 @@ black:
         tests/metadata.TestCase
         tests/ndk-release-checksums.py
         tests/rewritemeta.TestCase
+        tests/scanner.TestCase
         tests/signindex.TestCase
         tests/verify.TestCase
 

fdroidserver/scanner.py

@@ -136,7 +136,7 @@ def get_embedded_classes(apkfile, depth=0):
     if depth > 10:  # zipbomb protection
         return {_('Max recursion depth in ZIP file reached: %s') % apkfile}
 
-    apk_regex = re.compile(r'.*\.apk')
+    archive_regex = re.compile(r'.*\.(aab|aar|apk|apks|jar|war|xapk|zip)$')
     class_regex = re.compile(r'classes.*\.dex')
     classes = set()
 
@@ -144,11 +144,21 @@ def get_embedded_classes(apkfile, depth=0):
         with TemporaryDirectory() as tmp_dir, zipfile.ZipFile(apkfile, 'r') as apk_zip:
             for info in apk_zip.infolist():
                 # apk files can contain apk files, again
-                if apk_regex.search(info.filename):
                 with apk_zip.open(info) as apk_fp:
+                    if zipfile.is_zipfile(apk_fp):
                         classes = classes.union(get_embedded_classes(apk_fp, depth + 1))
+                        if not archive_regex.search(info.filename):
+                            classes.add(
+                                'ZIP file without proper file extension: %s'
+                                % info.filename
+                            )
+                        continue
 
-                elif class_regex.search(info.filename):
+                with apk_zip.open(info.filename) as fp:
+                    file_magic = fp.read(3)
+                if file_magic == b'dex':
+                    if not class_regex.search(info.filename):
+                        classes.add('DEX file with fake name: %s' % info.filename)
                     apk_zip.extract(info, tmp_dir)
                     run = common.SdkToolsPopen(
                         ["dexdump", '{}/{}'.format(tmp_dir, info.filename)],

tests/scanner.TestCase

@@ -13,6 +13,7 @@ import textwrap
 import unittest
 import uuid
 import yaml
+import zipfile
 import collections
 import pathlib
 from unittest import mock
@@ -188,26 +189,37 @@ class ScannerTest(unittest.TestCase):
             for msg, f in fdroidserver.scanner.json_per_build[section]:
                 files[section].append(f)
 
-        self.assertFalse('ascii.out' in files['errors'],
+        self.assertFalse(
-                         'an ASCII .out file is not an error')
+            'ascii.out' in files['errors'], 'ASCII .out file is not an error'
-        self.assertFalse('snippet.png' in files['errors'],
+        )
-                         'an executable valid image is not an error')
+        self.assertFalse(
+            'snippet.png' in files['errors'], 'executable valid image is not an error'
+        )
 
         self.assertTrue('arg.jar' in files['errors'], 'all JAR files are errors')
         self.assertTrue('baz.so' in files['errors'], 'all .so files are errors')
-        self.assertTrue('binary.out' in files['errors'], 'a binary .out file is an error')
+        self.assertTrue(
-        self.assertTrue('classes.dex' in files['errors'], 'all classes.dex files are errors')
+            'binary.out' in files['errors'], 'a binary .out file is an error'
+        )
+        self.assertTrue(
+            'classes.dex' in files['errors'], 'all classes.dex files are errors'
+        )
         self.assertTrue('sqlcipher.aar' in files['errors'], 'all AAR files are errors')
         self.assertTrue('static.a' in files['errors'], 'all .a files are errors')
 
-        self.assertTrue('fake.png' in files['warnings'],
+        self.assertTrue(
-                        'a random binary that is executable that is not an image is a warning')
+            'fake.png' in files['warnings'],
-        self.assertTrue('src/test/resources/classes.dex' in files['warnings'],
+            'a random binary that is executable that is not an image is a warning',
-                        'suspicious file but in a test dir is a warning')
+        )
+        self.assertTrue(
+            'src/test/resources/classes.dex' in files['warnings'],
+            'suspicious file but in a test dir is a warning',
+        )
 
         for f in remove:
-            self.assertTrue(f in files['infos'],
+            self.assertTrue(
-                            f + ' should be removed with an info message')
+                f in files['infos'], '%s should be removed with an info message' % f
+            )
 
     def test_build_local_scanner(self):
         """`fdroid build` calls scanner functions, test them here"""
@@ -260,14 +272,26 @@ class ScannerTest(unittest.TestCase):
 
         with mock.patch('fdroidserver.common.replace_build_vars', wraps=make_fake_apk):
             with mock.patch('fdroidserver.common.get_native_code', return_value='x86'):
-                with mock.patch('fdroidserver.common.get_apk_id',
+                with mock.patch(
-                                return_value=(app.id, build.versionCode, build.versionName)):
+                    'fdroidserver.common.get_apk_id',
-                    with mock.patch('fdroidserver.common.is_apk_and_debuggable', return_value=False):
+                    return_value=(app.id, build.versionCode, build.versionName),
+                ):
+                    with mock.patch(
+                        'fdroidserver.common.is_apk_and_debuggable', return_value=False
+                    ):
                         fdroidserver.build.build_local(
-                            app, build, vcs,
+                            app,
-                            build_dir=testdir, output_dir=testdir,
+                            build,
-                            log_dir=None, srclib_dir=None, extlib_dir=None, tmp_dir=None,
+                            vcs,
-                            force=False, onserver=False, refresh=False
+                            build_dir=testdir,
+                            output_dir=testdir,
+                            log_dir=None,
+                            srclib_dir=None,
+                            extlib_dir=None,
+                            tmp_dir=None,
+                            force=False,
+                            onserver=False,
+                            refresh=False,
                         )
         self.assertTrue(os.path.exists('baz.so'))
         self.assertTrue(os.path.exists('foo.aar'))
@@ -314,9 +338,107 @@ class ScannerTest(unittest.TestCase):
         self.assertFalse(os.path.exists("build.gradle"))
         self.assertEqual(0, count, 'there should be this many errors')
 
+    def test_get_embedded_classes(self):
+        config = dict()
+        fdroidserver.common.config = config
+        fdroidserver.common.fill_config_defaults(config)
+        for f in (
+            'apk.embedded_1.apk',
+            'bad-unicode-πÇÇ现代通用字-български-عربي1.apk',
+            'janus.apk',
+            'minimal_targetsdk_30_unsigned.apk',
+            'no_targetsdk_minsdk1_unsigned.apk',
+            'org.bitbucket.tickytacky.mirrormirror_1.apk',
+            'org.bitbucket.tickytacky.mirrormirror_2.apk',
+            'org.bitbucket.tickytacky.mirrormirror_3.apk',
+            'org.bitbucket.tickytacky.mirrormirror_4.apk',
+            'org.dyndns.fules.ck_20.apk',
+            'SpeedoMeterApp.main_1.apk',
+            'urzip.apk',
+            'urzip-badcert.apk',
+            'urzip-badsig.apk',
+            'urzip-release.apk',
+            'urzip-release-unsigned.apk',
+            'repo/com.example.test.helloworld_1.apk',
+            'repo/com.politedroid_3.apk',
+            'repo/com.politedroid_4.apk',
+            'repo/com.politedroid_5.apk',
+            'repo/com.politedroid_6.apk',
+            'repo/duplicate.permisssions_9999999.apk',
+            'repo/info.zwanenburg.caffeinetile_4.apk',
+            'repo/no.min.target.sdk_987.apk',
+            'repo/obb.main.oldversion_1444412523.apk',
+            'repo/obb.mainpatch.current_1619_another-release-key.apk',
+            'repo/obb.mainpatch.current_1619.apk',
+            'repo/obb.main.twoversions_1101613.apk',
+            'repo/obb.main.twoversions_1101615.apk',
+            'repo/obb.main.twoversions_1101617.apk',
+            'repo/souch.smsbypass_9.apk',
+            'repo/urzip-; Рахма́, [rɐxˈmanʲɪnəf] سيرجي_رخمانينوف 谢·.apk',
+            'repo/v1.v2.sig_1020.apk',
+        ):
+            self.assertNotEqual(
+                set(),
+                fdroidserver.scanner.get_embedded_classes(f),
+                'should return results for ' + f,
+            )
+
+    def test_get_embedded_classes_empty_archives(self):
+        config = dict()
+        fdroidserver.common.config = config
+        fdroidserver.common.fill_config_defaults(config)
+        print('basedir')
+        for f in (
+            'Norway_bouvet_europe_2.obf.zip',
+            'repo/fake.ota.update_1234.zip',
+        ):
+            self.assertEqual(
+                set(),
+                fdroidserver.scanner.get_embedded_classes(f),
+                'should return not results for ' + f,
+            )
+
+    @unittest.skipIf(
+        sys.hexversion < 0x03090000, 'Python < 3.9 has a limited zipfile.is_zipfile()'
+    )
+    def test_get_embedded_classes_secret_apk(self):
+        """Try to hide an APK+DEX in an APK and see if we can find it"""
+        config = dict()
+        fdroidserver.common.config = config
+        fdroidserver.common.fill_config_defaults(config)
+        apk = 'urzip.apk'
+        mapzip = 'Norway_bouvet_europe_2.obf.zip'
+        secretfile = os.path.join(
+            self.basedir, 'org.bitbucket.tickytacky.mirrormirror_1.apk'
+        )
+        with tempfile.TemporaryDirectory() as tmpdir:
+            shutil.copy(apk, tmpdir)
+            shutil.copy(mapzip, tmpdir)
+            os.chdir(tmpdir)
+            with zipfile.ZipFile(mapzip, 'a') as zipfp:
+                zipfp.write(secretfile, 'secretapk')
+                with zipfile.ZipFile(apk) as readfp:
+                    with readfp.open('classes.dex') as cfp:
+                        zipfp.writestr('secretdex', cfp.read())
+            with zipfile.ZipFile(apk, 'a') as zipfp:
+                zipfp.write(mapzip)
+
+            cls = fdroidserver.scanner.get_embedded_classes(apk)
+            self.assertTrue(
+                'org/bitbucket/tickytacky/mirrormirror/MainActivity' in cls,
+                'this should find the classes in the hidden, embedded APK',
+            )
+            self.assertTrue(
+                'DEX file with fake name: secretdex' in cls,
+                'badly named embedded DEX fils should throw an error',
+            )
+            self.assertTrue(
+                'ZIP file without proper file extension: secretapk' in cls,
+                'badly named embedded ZIPs should throw an error',
+            )
+
 
 class Test_scan_binary(unittest.TestCase):
-
     def setUp(self):
         self.basedir = os.path.join(localmodule, 'tests')
         config = dict()
@@ -326,51 +448,68 @@ class Test_scan_binary(unittest.TestCase):
 
     def test_code_signature_match(self):
         apkfile = os.path.join(self.basedir, 'no_targetsdk_minsdk1_unsigned.apk')
-        with mock.patch("fdroidserver.scanner.CODE_SIGNATURES", {"java/lang/Object": re.compile(
+        mock_code_signatures = {
+            "java/lang/Object": re.compile(
                 r'.*java/lang/Object', re.IGNORECASE | re.UNICODE
-        )}):
+            )
+        }
+        with mock.patch("fdroidserver.scanner.CODE_SIGNATURES", mock_code_signatures):
             self.assertEqual(
                 1,
                 fdroidserver.scanner.scan_binary(apkfile),
-                "Did not find expected code signature '{}' in binary '{}'".format(fdroidserver.scanner.CODE_SIGNATURES.values(), apkfile),
+                "Did not find expected code signature '{}' in binary '{}'".format(
+                    fdroidserver.scanner.CODE_SIGNATURES.values(), apkfile
+                ),
             )
 
     @unittest.skipIf(
         sys.version_info < (3, 9),
         "Our implementation for traversing zip files will silently fail to work"
         "on older python versions, also see: "
-        "https://gitlab.com/fdroid/fdroidserver/-/merge_requests/1110#note_932026766"
+        "https://gitlab.com/fdroid/fdroidserver/-/merge_requests/1110#note_932026766",
     )
     def test_bottom_level_embedded_apk_code_signature(self):
         apkfile = os.path.join(self.basedir, 'apk.embedded_1.apk')
-        with mock.patch("fdroidserver.scanner.CODE_SIGNATURES", {"org/bitbucket/tickytacky/mirrormirror/MainActivity": re.compile(
+        mock_code_signatures = {
-            r'.*org/bitbucket/tickytacky/mirrormirror/MainActivity', re.IGNORECASE | re.UNICODE
+            "org/bitbucket/tickytacky/mirrormirror/MainActivity": re.compile(
-        )}):
+                r'.*org/bitbucket/tickytacky/mirrormirror/MainActivity',
+                re.IGNORECASE | re.UNICODE,
+            )
+        }
+        with mock.patch("fdroidserver.scanner.CODE_SIGNATURES", mock_code_signatures):
             self.assertEqual(
                 1,
                 fdroidserver.scanner.scan_binary(apkfile),
-                "Did not find expected code signature '{}' in binary '{}'".format(fdroidserver.scanner.CODE_SIGNATURES.values(), apkfile),
+                "Did not find expected code signature '{}' in binary '{}'".format(
+                    fdroidserver.scanner.CODE_SIGNATURES.values(), apkfile
+                ),
             )
 
     def test_top_level_signature_embedded_apk_present(self):
         apkfile = os.path.join(self.basedir, 'apk.embedded_1.apk')
-        with mock.patch("fdroidserver.scanner.CODE_SIGNATURES", {"org/fdroid/ci/BuildConfig": re.compile(
+        mock_code_signatures = {
+            "org/fdroid/ci/BuildConfig": re.compile(
                 r'.*org/fdroid/ci/BuildConfig', re.IGNORECASE | re.UNICODE
-        )}):
+            )
+        }
+        with mock.patch("fdroidserver.scanner.CODE_SIGNATURES", mock_code_signatures):
             self.assertEqual(
                 1,
                 fdroidserver.scanner.scan_binary(apkfile),
-                "Did not find expected code signature '{}' in binary '{}'".format(fdroidserver.scanner.CODE_SIGNATURES.values(), apkfile),
+                "Did not find expected code signature '{}' in binary '{}'".format(
+                    fdroidserver.scanner.CODE_SIGNATURES.values(), apkfile
+                ),
             )
 
     def test_no_match(self):
         apkfile = os.path.join(self.basedir, 'no_targetsdk_minsdk1_unsigned.apk')
         result = fdroidserver.scanner.scan_binary(apkfile)
-        self.assertEqual(0, result, "Found false positives in binary '{}'".format(apkfile))
+        self.assertEqual(
+            0, result, "Found false positives in binary '{}'".format(apkfile)
+        )
 
 
 class Test__exodus_compile_signatures(unittest.TestCase):
-
     def setUp(self):
         self.m1 = mock.Mock()
         self.m1.code_signature = r"^random\sregex$"
@@ -380,10 +519,13 @@ class Test__exodus_compile_signatures(unittest.TestCase):
 
     def test_ok(self):
         result = fdroidserver.scanner._exodus_compile_signatures(self.mock_sigs)
-        self.assertListEqual(result, [
+        self.assertListEqual(
+            result,
+            [
                 re.compile(self.m1.code_signature),
                 re.compile(self.m2.code_signature),
-        ])
+            ],
+        )
 
     def test_not_iterable(self):
         result = fdroidserver.scanner._exodus_compile_signatures(123)
@@ -391,10 +533,10 @@ class Test__exodus_compile_signatures(unittest.TestCase):
 
 
 class Test_load_exodus_trackers_signatures(unittest.TestCase):
-
     def setUp(self):
         self.requests_ret = mock.Mock()
-        self.requests_ret.json = mock.Mock(return_value={
+        self.requests_ret.json = mock.Mock(
+            return_value={
                 "trackers": {
                     "1": {
                         "id": 1,
@@ -417,9 +559,10 @@ class Test_load_exodus_trackers_signatures(unittest.TestCase):
                         "website": "https://pst.com",
                         "categories": ["tracker"],
                         "documentation": [],
-                }
                     },
-        })
+                },
+            }
+        )
         self.requests_func = mock.Mock(return_value=self.requests_ret)
         self.compilesig_func = mock.Mock(return_value="mocked return value")
 
@@ -427,19 +570,18 @@ class Test_load_exodus_trackers_signatures(unittest.TestCase):
         with mock.patch("requests.get", self.requests_func), mock.patch(
             "fdroidserver.scanner._exodus_compile_signatures", self.compilesig_func
         ):
-            result_sigs, result_regex = fdroidserver.scanner.load_exodus_trackers_signatures()
+            sigs, regex = fdroidserver.scanner.load_exodus_trackers_signatures()
             self.requests_func.assert_called_once_with(
                 "https://reports.exodus-privacy.eu.org/api/trackers", timeout=300
             )
-            self.assertEqual(len(result_sigs), 2)
+            self.assertEqual(len(sigs), 2)
-            self.assertListEqual([1, 2], sorted([x.id for x in result_sigs]))
+            self.assertListEqual([1, 2], sorted([x.id for x in sigs]))
 
-            self.compilesig_func.assert_called_once_with(result_sigs)
+            self.compilesig_func.assert_called_once_with(sigs)
-            self.assertEqual(result_regex, "mocked return value")
+            self.assertEqual(regex, "mocked return value")
 
 
 class Test_main(unittest.TestCase):
-
     def setUp(self):
         self.args = ["com.example.app", "local/additional.apk", "another.apk"]
         self.exit_func = mock.Mock()
@@ -456,7 +598,9 @@ class Test_main(unittest.TestCase):
             "sys.exit", self.exit_func
         ), mock.patch("sys.argv", ["fdroid scanner", *self.args]), mock.patch(
             "fdroidserver.common.read_app_args", self.read_app_args_func
-        ), mock.patch("fdroidserver.scanner.scan_binary", self.scan_binary_func):
+        ), mock.patch(
+            "fdroidserver.scanner.scan_binary", self.scan_binary_func
+        ):
             fdroidserver.scanner.main()
 
             self.exit_func.assert_not_called()
@@ -475,7 +619,9 @@ class Test_main(unittest.TestCase):
             "sys.exit", self.exit_func
         ), mock.patch("sys.argv", ["fdroid scanner", *self.args]), mock.patch(
             "fdroidserver.common.read_app_args", self.read_app_args_func
-        ), mock.patch("fdroidserver.scanner.scan_binary", self.scan_binary_func):
+        ), mock.patch(
+            "fdroidserver.scanner.scan_binary", self.scan_binary_func
+        ):
             pathlib.Path(self.args[0]).touch()
             fdroidserver.scanner.main()
 
@@ -498,11 +644,13 @@ if __name__ == "__main__":
     (fdroidserver.common.options, args) = parser.parse_args(['--verbose'])
 
     newSuite = unittest.TestSuite()
-    newSuite.addTests([
+    newSuite.addTests(
+        [
             unittest.makeSuite(ScannerTest),
             unittest.makeSuite(Test_scan_binary),
             unittest.makeSuite(Test__exodus_compile_signatures),
             unittest.makeSuite(Test_load_exodus_trackers_signatures),
             unittest.makeSuite(Test_main),
-    ])
+        ]
+    )
     unittest.main(failfast=False)