update: reject APKs with invalid file sig, probably Janus exploits

This just checks the first four bytes of the APK file, aka the "file
signature", to make sure it is the ZIP signature and not the DEX signature.
This was checked against the test APK, and I ran it against some known
malware and all of f-droid.org to make sure it works.

All valid ZIP files (therefore APK files) should start with the ZIP
Local File Header of four bytes.

https://www.guardsquare.com/en/blog/new-android-vulnerability-allows-attackers-modify-apps-without-affecting-their-signatures

diff --git a/fdroidserver/update.py b/fdroidserver/update.py
index e548df43d845540d5d61de3fff9814cb361fd8a8..f90e49932258315d985a2286bcf76a7fe674cdb5 100644 (file)
--- a/fdroidserver/update.py
+++ b/fdroidserver/update.py
@@ -496,8 +496,10 @@ def has_known_vulnerability(filename):
 
     Checks whether there are more than one classes.dex or AndroidManifest.xml
     files, which is invalid and an essential part of the "Master Key" attack.
-
     http://www.saurik.com/id/17
+
+    Janus is similar to Master Key but is perhaps easier to scan for.
+    https://www.guardsquare.com/en/blog/new-android-vulnerability-allows-attackers-modify-apps-without-affecting-their-signatures
     """
 
     found_vuln = False
@@ -506,6 +508,13 @@ def has_known_vulnerability(filename):
     if not hasattr(has_known_vulnerability, "pattern"):
         has_known_vulnerability.pattern = re.compile(b'.*OpenSSL ([01][0-9a-z.-]+)')
 
+    with open(filename.encode(), 'rb') as fp:
+        first4 = fp.read(4)
+    if first4 != b'\x50\x4b\x03\x04':
+        raise FDroidException(_('{path} has bad file signature "{pattern}", possible Janus exploit!')
+                              .format(path=filename, pattern=first4.decode().replace('\n', ' ')) + '\n'
+                              + 'https://www.guardsquare.com/en/blog/new-android-vulnerability-allows-attackers-modify-apps-without-affecting-their-signatures')
+
     files_in_apk = set()
     with zipfile.ZipFile(filename) as zf:
         for name in zf.namelist():

diff --git a/tests/janus.apk b/tests/janus.apk
new file mode 100644 (file)
index 0000000..ed4eed9
Binary files /dev/null and b/tests/janus.apk differ

diff --git a/tests/run-tests b/tests/run-tests
index 696bcd75eaaba7ef05657e7ffad0d2cf3a0508da..af29f471eeb186a5480815658759b206b16afc5a 100755 (executable)
--- a/tests/run-tests
+++ b/tests/run-tests
@@ -9,7 +9,7 @@ echo_header() {
 copy_apks_into_repo() {
     set +x
     find $APKDIR -type f -name '*.apk' -print0 | while IFS= read -r -d '' f; do
-        echo $f | grep -F -v -e unaligned -e unsigned -e badsig -e badcert -e bad-unicode || continue
+        echo $f | grep -F -v -e unaligned -e unsigned -e badsig -e badcert -e bad-unicode -e janus.apk || continue
         apk=`$aapt dump badging "$f" | sed -n "s,^package: name='\(.*\)' versionCode='\([0-9][0-9]*\)' .*,\1_\2.apk,p"`
         test "$f" -nt repo/$apk && rm -f repo/$apk  # delete existing if $f is newer
         if [ ! -e repo/$apk ] && [ ! -e archive/$apk ]; then

diff --git a/tests/update.TestCase b/tests/update.TestCase
index e49e1bf09712860a2f72d72a17ff794aee439d72..db463a89ef543bd13bcdd88d5d72edd152575f0e 100755 (executable)
--- a/tests/update.TestCase
+++ b/tests/update.TestCase
@@ -601,6 +601,35 @@ class UpdateTest(unittest.TestCase):
         self.assertEqual('urzip', data['Name'])
         self.assertEqual('urzip', data['Summary'])
 
+    def test_has_known_vulnerability(self):
+        good = [
+            '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',
+            'urzip.apk',
+            'urzip-badcert.apk',
+            'urzip-badsig.apk',
+            'urzip-release.apk',
+            'urzip-release-unsigned.apk',
+            'repo/com.politedroid_3.apk',
+            'repo/com.politedroid_4.apk',
+            'repo/com.politedroid_5.apk',
+            'repo/com.politedroid_6.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/urzip-; Рахма́нинов, [rɐxˈmanʲɪnəf] سيرجي_رخمانينوف 谢尔盖·.apk',
+        ]
+        for f in good:
+            self.assertFalse(fdroidserver.update.has_known_vulnerability(f))
+        with self.assertRaises(fdroidserver.exception.FDroidException):
+            fdroidserver.update.has_known_vulnerability('janus.apk')
+
 
 if __name__ == "__main__":
     parser = optparse.OptionParser()