From 4197455436b4680c5099ee5dc0c1791c95283744 Mon Sep 17 00:00:00 2001 From: Jochen Sprickerhof Date: Tue, 10 May 2022 14:54:32 +0200 Subject: [PATCH 1/3] Support more file types in get_embedded_classes Closes: #999 --- fdroidserver/scanner.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fdroidserver/scanner.py b/fdroidserver/scanner.py index 7bc28444..85688cfb 100644 --- a/fdroidserver/scanner.py +++ b/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,7 +144,7 @@ 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): + if archive_regex.search(info.filename): with apk_zip.open(info) as apk_fp: classes = classes.union(get_embedded_classes(apk_fp, depth + 1)) From aa190d532fa72062d0dbdc4bbadee825e36e98df Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 22 Sep 2022 11:51:59 +0200 Subject: [PATCH 2/3] scanner.TestCase: manually convert to black code format I manually changed some code structures to give a decent code format. --- .gitlab-ci.yml | 1 + tests/scanner.TestCase | 208 +++++++++++++++++++++++++---------------- 2 files changed, 129 insertions(+), 80 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index baa7ed82..e4c9a813 100644 --- a/.gitlab-ci.yml +++ b/.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 diff --git a/tests/scanner.TestCase b/tests/scanner.TestCase index 65b2a8fd..0dac2fb1 100755 --- a/tests/scanner.TestCase +++ b/tests/scanner.TestCase @@ -188,26 +188,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'], - 'an ASCII .out file is not an error') - self.assertFalse('snippet.png' in files['errors'], - 'an executable valid image is not an error') + self.assertFalse( + 'ascii.out' in files['errors'], 'ASCII .out file 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('classes.dex' in files['errors'], 'all classes.dex files are errors') + self.assertTrue( + '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'], - 'a random binary that is executable that is not an image is a warning') - self.assertTrue('src/test/resources/classes.dex' in files['warnings'], - 'suspicious file but in a test dir is a warning') + self.assertTrue( + 'fake.png' in files['warnings'], + 'a random binary that is executable that is not an image 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'], - f + ' should be removed with an info message') + self.assertTrue( + 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 +271,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', - return_value=(app.id, build.versionCode, build.versionName)): - with mock.patch('fdroidserver.common.is_apk_and_debuggable', return_value=False): + with mock.patch( + 'fdroidserver.common.get_apk_id', + 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, - build_dir=testdir, output_dir=testdir, - log_dir=None, srclib_dir=None, extlib_dir=None, tmp_dir=None, - force=False, onserver=False, refresh=False + app, + build, + vcs, + 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')) @@ -316,7 +339,6 @@ class ScannerTest(unittest.TestCase): class Test_scan_binary(unittest.TestCase): - def setUp(self): self.basedir = os.path.join(localmodule, 'tests') config = dict() @@ -326,51 +348,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( - r'.*java/lang/Object', re.IGNORECASE | re.UNICODE - )}): + 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( - r'.*org/bitbucket/tickytacky/mirrormirror/MainActivity', re.IGNORECASE | re.UNICODE - )}): + mock_code_signatures = { + "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( - r'.*org/fdroid/ci/BuildConfig', re.IGNORECASE | re.UNICODE - )}): + 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 +419,13 @@ class Test__exodus_compile_signatures(unittest.TestCase): def test_ok(self): result = fdroidserver.scanner._exodus_compile_signatures(self.mock_sigs) - self.assertListEqual(result, [ - re.compile(self.m1.code_signature), - re.compile(self.m2.code_signature), - ]) + 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,35 +433,36 @@ 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={ - "trackers": { - "1": { - "id": 1, - "name": "Steyer Puch 1", - "description": "blah blah blah", - "creation_date": "1956-01-01", - "code_signature": "com.puch.|com.steyer.", - "network_signature": "pst\\.com", - "website": "https://pst.com", - "categories": ["tracker"], - "documentation": [], + self.requests_ret.json = mock.Mock( + return_value={ + "trackers": { + "1": { + "id": 1, + "name": "Steyer Puch 1", + "description": "blah blah blah", + "creation_date": "1956-01-01", + "code_signature": "com.puch.|com.steyer.", + "network_signature": "pst\\.com", + "website": "https://pst.com", + "categories": ["tracker"], + "documentation": [], + }, + "2": { + "id": 2, + "name": "Steyer Puch 2", + "description": "blah blah blah", + "creation_date": "1956-01-01", + "code_signature": "com.puch.|com.steyer.", + "network_signature": "pst\\.com", + "website": "https://pst.com", + "categories": ["tracker"], + "documentation": [], + }, }, - "2": { - "id": 2, - "name": "Steyer Puch 2", - "description": "blah blah blah", - "creation_date": "1956-01-01", - "code_signature": "com.puch.|com.steyer.", - "network_signature": "pst\\.com", - "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 +470,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.assertListEqual([1, 2], sorted([x.id for x in result_sigs])) + self.assertEqual(len(sigs), 2) + self.assertListEqual([1, 2], sorted([x.id for x in sigs])) - self.compilesig_func.assert_called_once_with(result_sigs) - self.assertEqual(result_regex, "mocked return value") + self.compilesig_func.assert_called_once_with(sigs) + 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 +498,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 +519,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 +544,13 @@ if __name__ == "__main__": (fdroidserver.common.options, args) = parser.parse_args(['--verbose']) newSuite = unittest.TestSuite() - 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), - ]) + 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) From 3de6063a01ec2242b80023f76d6dc04030855e1a Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Thu, 29 Sep 2022 17:21:03 +0200 Subject: [PATCH 3/3] scanner: open DEX/ZIP by file magic; throw errors on bad filenames --- fdroidserver/scanner.py | 16 +++++-- tests/scanner.TestCase | 100 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 3 deletions(-) diff --git a/fdroidserver/scanner.py b/fdroidserver/scanner.py index 85688cfb..710d0c23 100644 --- a/fdroidserver/scanner.py +++ b/fdroidserver/scanner.py @@ -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 archive_regex.search(info.filename): - with apk_zip.open(info) as apk_fp: + 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)], diff --git a/tests/scanner.TestCase b/tests/scanner.TestCase index 0dac2fb1..991cd9f0 100755 --- a/tests/scanner.TestCase +++ b/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 @@ -337,6 +338,105 @@ 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):