Merge branch 'minor-security-fixups' into 'master'

Minor security fixups

Closes #163

See merge request fdroid/fdroidserver!570

buildserver/provision-apt-get-install

@@ -89,6 +89,7 @@ packages="
  python-lxml
  python-magic
  python-setuptools
+ python3-defusedxml
  python3-git/jessie-backports
  python3-gitdb/jessie-backports
  python3-gnupg

fdroidserver/common.py

@@ -39,7 +39,12 @@ import base64
 import zipfile
 import tempfile
 import json
-import defusedxml.ElementTree as XMLElementTree
+
+# TODO change to only import defusedxml once its installed everywhere
+try:
+    import defusedxml.ElementTree as XMLElementTree
+except ImportError:
+    import xml.etree.ElementTree as XMLElementTree  # nosec this is a fallback only
 
 from binascii import hexlify
 from datetime import datetime, timedelta
@@ -1511,14 +1516,26 @@ def parse_androidmanifests(paths, app):
     if max_version is None:
         max_version = "Unknown"
 
-    if max_package and not is_valid_package_name(max_package):
+    if max_package and not is_valid_java_package_name(max_package):
         raise FDroidException(_("Invalid package name {0}").format(max_package))
 
     return (max_version, max_vercode, max_package)
 
 
 def is_valid_package_name(name):
-    return re.match("[A-Za-z_][A-Za-z_0-9.]+$", name)
+    """Check whether name is a valid fdroid package name
+
+    APKs and manually defined package names must use a valid Java
+    Package Name.  Automatically generated package names for non-APK
+    files use the SHA-256 sum.
+
+    """
+    return re.match("^([a-f0-9]+|[A-Za-z_][A-Za-z_0-9.]+)$", name)
+
+
+def is_valid_java_package_name(name):
+    """Check whether name is a valid Java package name aka Application ID"""
+    return re.match("^[A-Za-z_][A-Za-z_0-9.]+$", name)
 
 
 def getsrclib(spec, srclib_dir, subdir=None, basepath=False,

fdroidserver/metadata.py

@@ -805,6 +805,9 @@ def read_metadata(xref=True, check_vcs=[], refresh=True, sort_by_time=False):
         if metadatapath == '.fdroid.txt':
             warn_or_exception(_('.fdroid.txt is not supported!  Convert to .fdroid.yml or .fdroid.json.'))
         appid, _ignored = fdroidserver.common.get_extension(os.path.basename(metadatapath))
+        if appid != '.fdroid' and not fdroidserver.common.is_valid_package_name(appid):
+            warn_or_exception(_("{appid} from {path} is not a valid Java Package Name!")
+                              .format(appid=appid, path=metadatapath))
         if appid in apps:
             warn_or_exception(_("Found multiple metadata files for {appid}")
                               .format(appid=appid))

fdroidserver/update.py

@@ -1064,6 +1064,10 @@ def scan_apk(apk_file):
     else:
         scan_apk_aapt(apk, apk_file)
 
+    if not common.is_valid_java_package_name(apk['packageName']):
+        raise BuildException(_("{appid} from {path} is not a valid Java Package Name!")
+                             .format(appid=apk['packageName'], path=apk_file))
+
     # Get the signature, or rather the signing key fingerprints
     logging.debug('Getting signature of {0}'.format(os.path.basename(apk_file)))
     apk['sig'] = getsig(apk_file)

tests/common.TestCase

@@ -158,9 +158,10 @@ class CommonTest(unittest.TestCase):
             self.assertFalse(debuggable,
                              "debuggable APK state was not properly parsed!")
 
-    def testPackageNameValidity(self):
+    def test_is_valid_package_name(self):
         for name in ["org.fdroid.fdroid",
-                     "org.f_droid.fdr0ID"]:
+                     "org.f_droid.fdr0ID",
+                     "05041684efd9b16c2888b1eddbadd0359f655f311b89bdd1737f560a10d20fb8"]:
             self.assertTrue(fdroidserver.common.is_valid_package_name(name),
                             "{0} should be a valid package name".format(name))
         for name in ["0rg.fdroid.fdroid",
@@ -170,6 +171,19 @@ class CommonTest(unittest.TestCase):
             self.assertFalse(fdroidserver.common.is_valid_package_name(name),
                              "{0} should not be a valid package name".format(name))
 
+    def test_is_valid_java_package_name(self):
+        for name in ["org.fdroid.fdroid",
+                     "org.f_droid.fdr0ID"]:
+            self.assertTrue(fdroidserver.common.is_valid_java_package_name(name),
+                            "{0} should be a valid package name".format(name))
+        for name in ["0rg.fdroid.fdroid",
+                     ".f_droid.fdr0ID",
+                     "org.fdroid/fdroid",
+                     "/org.fdroid.fdroid",
+                     "05041684efd9b16c2888b1eddbadd0359f655f311b89bdd1737f560a10d20fb8"]:
+            self.assertFalse(fdroidserver.common.is_valid_java_package_name(name),
+                             "{0} should not be a valid package name".format(name))
+
     def test_prepare_sources(self):
         testint = 99999999
         teststr = 'FAKE_STR_FOR_TESTING'