diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 71568d71..96ba1649 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -119,16 +119,22 @@ pip_install: - fdroid readmeta - fdroid update --help -lint_format_safety_checks: +lint_format_safety_bandit_checks: image: alpine:3.7 variables: LANG: C.UTF-8 script: - apk add --no-cache bash dash ca-certificates python3 - python3 -m ensurepip - - pip3 install pycodestyle pyflakes 'pylint<2.0' safety + - pip3 install bandit pycodestyle pyflakes 'pylint<2.0' safety - export EXITVALUE=0 - ./hooks/pre-commit || export EXITVALUE=1 + - bandit + -ii + -s B110,B310,B322,B404,B408,B410,B603,B607 + -x fdroidserver/dscanner.py,docker/install_agent.py,docker/drozer.py + -r $CI_PROJECT_DIR + || export EXITVALUE=1 - safety check --full-report || export EXITVALUE=1 - pylint --rcfile=.pylint-rcfile --output-format=colorized --reports=n fdroid diff --git a/fdroidserver/btlog.py b/fdroidserver/btlog.py index de357039..43ea2313 100755 --- a/fdroidserver/btlog.py +++ b/fdroidserver/btlog.py @@ -28,6 +28,7 @@ import collections +import defusedxml.minidom import git import glob import os @@ -36,7 +37,6 @@ import logging import requests import shutil import tempfile -import xml.dom.minidom import zipfile from argparse import ArgumentParser @@ -94,7 +94,7 @@ For more info on this idea: continue dest = os.path.join(cpdir, f) if f.endswith('.xml'): - doc = xml.dom.minidom.parse(repof) + doc = defusedxml.minidom.parse(repof) output = doc.toprettyxml(encoding='utf-8') with open(dest, 'wb') as f: f.write(output) diff --git a/fdroidserver/build.py b/fdroidserver/build.py index 7dbc326d..a5ca66c6 100644 --- a/fdroidserver/build.py +++ b/fdroidserver/build.py @@ -21,6 +21,7 @@ import os import shutil import glob import subprocess +import posixpath import re import resource import sys @@ -92,7 +93,7 @@ def build_server(app, build, vcs, build_dir, output_dir, log_dir, force): port=sshinfo['port'], timeout=300, look_for_keys=False, key_filename=sshinfo['idfile']) - homedir = '/home/' + sshinfo['user'] + homedir = posixpath.join('/home', sshinfo['user']) # Get an SFTP connection... ftp = sshs.open_sftp() @@ -126,8 +127,8 @@ def build_server(app, build, vcs, build_dir, output_dir, log_dir, force): ftp.chdir('fdroidserver') ftp.put(os.path.join(serverpath, '..', 'fdroid'), 'fdroid') ftp.put(os.path.join(serverpath, '..', 'gradlew-fdroid'), 'gradlew-fdroid') - ftp.chmod('fdroid', 0o755) - ftp.chmod('gradlew-fdroid', 0o755) + ftp.chmod('fdroid', 0o755) # nosec B103 permissions are appropriate + ftp.chmod('gradlew-fdroid', 0o755) # nosec B103 permissions are appropriate send_dir(os.path.join(serverpath)) ftp.chdir(homedir) @@ -159,7 +160,7 @@ def build_server(app, build, vcs, build_dir, output_dir, log_dir, force): ftp.mkdir('srclib') # Copy any extlibs that are required... if build.extlibs: - ftp.chdir(homedir + '/build/extlib') + ftp.chdir(posixpath.join(homedir, 'build', 'extlib')) for lib in build.extlibs: lib = lib.strip() libsrc = os.path.join('build/extlib', lib) @@ -186,20 +187,20 @@ def build_server(app, build, vcs, build_dir, output_dir, log_dir, force): srclibpaths.append(basesrclib) for name, number, lib in srclibpaths: logging.info("Sending srclib '%s'" % lib) - ftp.chdir(homedir + '/build/srclib') + ftp.chdir(posixpath.join(homedir, 'build', 'srclib')) if not os.path.exists(lib): raise BuildException("Missing srclib directory '" + lib + "'") fv = '.fdroidvcs-' + name ftp.put(os.path.join('build/srclib', fv), fv) send_dir(lib) # Copy the metadata file too... - ftp.chdir(homedir + '/srclibs') + ftp.chdir(posixpath.join(homedir, 'srclibs')) ftp.put(os.path.join('srclibs', name + '.txt'), name + '.txt') # Copy the main app source code # (no need if it's a srclib) if (not basesrclib) and os.path.exists(build_dir): - ftp.chdir(homedir + '/build') + ftp.chdir(posixpath.join(homedir, 'build')) fv = '.fdroidvcs-' + app.id ftp.put(os.path.join('build', fv), fv) send_dir(build_dir) @@ -208,7 +209,7 @@ def build_server(app, build, vcs, build_dir, output_dir, log_dir, force): logging.info("Starting build...") chan = sshs.get_transport().open_session() chan.get_pty() - cmdline = os.path.join(homedir, 'fdroidserver', 'fdroid') + cmdline = posixpath.join(homedir, 'fdroidserver', 'fdroid') cmdline += ' build --on-server' if force: cmdline += ' --force --test' @@ -219,7 +220,7 @@ def build_server(app, build, vcs, build_dir, output_dir, log_dir, force): if options.notarball: cmdline += ' --no-tarball' cmdline += " %s:%s" % (app.id, build.versionCode) - chan.exec_command('bash --login -c "' + cmdline + '"') + chan.exec_command('bash --login -c "' + cmdline + '"') # nosec B601 inputs are sanitized # Fetch build process output ... try: @@ -255,7 +256,7 @@ def build_server(app, build, vcs, build_dir, output_dir, log_dir, force): # Retreive logs... toolsversion_log = common.get_toolsversion_logname(app, build) try: - ftp.chdir(os.path.join(homedir, log_dir)) + ftp.chdir(posixpath.join(homedir, log_dir)) ftp.get(toolsversion_log, os.path.join(log_dir, toolsversion_log)) logging.debug('retrieved %s', toolsversion_log) except Exception as e: @@ -264,9 +265,9 @@ def build_server(app, build, vcs, build_dir, output_dir, log_dir, force): # Retrieve the built files... logging.info("Retrieving build output...") if force: - ftp.chdir(homedir + '/tmp') + ftp.chdir(posixpath.join(homedir, 'tmp')) else: - ftp.chdir(homedir + '/unsigned') + ftp.chdir(posixpath.join(homedir, 'unsigned')) apkfile = common.get_release_filename(app, build) tarball = common.getsrcname(app, build) try: diff --git a/fdroidserver/checkupdates.py b/fdroidserver/checkupdates.py index 60314160..d8d6af43 100644 --- a/fdroidserver/checkupdates.py +++ b/fdroidserver/checkupdates.py @@ -57,7 +57,7 @@ def check_http(app): if not parsed.netloc or not parsed.scheme or parsed.scheme != 'https': raise FDroidException(_('UpdateCheckData has invalid URL: {url}').format(url=urlcode)) - vercode = "99999999" + vercode = None if len(urlcode) > 0: logging.debug("...requesting {0}".format(urlcode)) req = urllib.request.Request(urlcode, None) diff --git a/fdroidserver/common.py b/fdroidserver/common.py index 42aa0d5c..55cb2b65 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -39,7 +39,7 @@ import base64 import zipfile import tempfile import json -import xml.etree.ElementTree as XMLElementTree +import defusedxml.ElementTree as XMLElementTree from binascii import hexlify from datetime import datetime, timedelta @@ -71,7 +71,9 @@ CERT_PATH_REGEX = re.compile(r'^META-INF/.*\.(DSA|EC|RSA)$') APK_NAME_REGEX = re.compile(r'^([a-zA-Z][\w.]*)_(-?[0-9]+)_?([0-9a-f]{7})?\.apk') STANDARD_FILE_NAME_REGEX = re.compile(r'^(\w[\w.]*)_(-?[0-9]+)\.\w+') -XMLElementTree.register_namespace('android', 'http://schemas.android.com/apk/res/android') +MAX_VERSION_CODE = 0x7fffffff # Java's Integer.MAX_VALUE (2147483647) + +XMLNS_ANDROID = '{http://schemas.android.com/apk/res/android}' config = None options = None @@ -281,7 +283,7 @@ def read_config(opts, config_file='config.py'): logging.debug(_("Reading '{config_file}'").format(config_file=config_file)) with io.open(config_file, "rb") as f: code = compile(f.read(), config_file, 'exec') - exec(code, None, config) + exec(code, None, config) # nosec TODO switch to YAML file else: logging.warning(_("No 'config.py' found, using defaults.")) @@ -1287,9 +1289,9 @@ def fetch_real_name(app_dir, flavours): app = xml.find('application') if app is None: continue - if "{http://schemas.android.com/apk/res/android}label" not in app.attrib: + if XMLNS_ANDROID + "label" not in app.attrib: continue - label = app.attrib["{http://schemas.android.com/apk/res/android}label"] + label = app.attrib[XMLNS_ANDROID + "label"] result = retrieve_string_singleline(app_dir, label) if result: result = result.strip() @@ -1469,12 +1471,12 @@ def parse_androidmanifests(paths, app): s = xml.attrib["package"] if app_matches_packagename(app, s): package = s - if "{http://schemas.android.com/apk/res/android}versionName" in xml.attrib: - version = xml.attrib["{http://schemas.android.com/apk/res/android}versionName"] + if XMLNS_ANDROID + "versionName" in xml.attrib: + version = xml.attrib[XMLNS_ANDROID + "versionName"] base_dir = os.path.dirname(path) version = retrieve_string_singleline(base_dir, version) - if "{http://schemas.android.com/apk/res/android}versionCode" in xml.attrib: - a = xml.attrib["{http://schemas.android.com/apk/res/android}versionCode"] + if XMLNS_ANDROID + "versionCode" in xml.attrib: + a = xml.attrib[XMLNS_ANDROID + "versionCode"] if string_is_integer(a): vercode = a except Exception: @@ -3275,8 +3277,12 @@ def get_git_describe_link(): def calculate_math_string(expr): - ops = {ast.Add: operator.add, ast.Sub: operator.sub, - ast.Mult: operator.mul} + ops = { + ast.Add: operator.add, + ast.Mult: operator.mul, + ast.Sub: operator.sub, + ast.USub: operator.neg, + } def execute_ast(node): if isinstance(node, ast.Num): # @@ -3285,7 +3291,7 @@ def calculate_math_string(expr): return ops[type(node.op)](execute_ast(node.left), execute_ast(node.right)) elif isinstance(node, ast.UnaryOp): # e.g., -1 - return ops[type(node.op)](eval(node.operand)) + return ops[type(node.op)](ast.literal_eval(node.operand)) else: raise SyntaxError(node) diff --git a/fdroidserver/publish.py b/fdroidserver/publish.py index a63e5c43..6561a9b8 100644 --- a/fdroidserver/publish.py +++ b/fdroidserver/publish.py @@ -60,12 +60,12 @@ def key_alias(appid): # For this particular app, the key alias is overridden... keyalias = config['keyaliases'][appid] if keyalias.startswith('@'): - m = hashlib.md5() + m = hashlib.md5() # nosec just used to generate a keyalias m.update(keyalias[1:].encode('utf-8')) keyalias = m.hexdigest()[:8] return keyalias else: - m = hashlib.md5() + m = hashlib.md5() # nosec just used to generate a keyalias m.update(appid.encode('utf-8')) return m.hexdigest()[:8] @@ -197,7 +197,7 @@ def main(): vercodes = common.read_pkg_args(options.appid, True) allaliases = [] for appid in allapps: - m = hashlib.md5() + m = hashlib.md5() # nosec just used to generate a keyalias m.update(appid.encode('utf-8')) keyalias = m.hexdigest()[:8] if keyalias in allaliases: @@ -307,11 +307,11 @@ def main(): # For this particular app, the key alias is overridden... keyalias = config['keyaliases'][appid] if keyalias.startswith('@'): - m = hashlib.md5() + m = hashlib.md5() # nosec just used to generate a keyalias m.update(keyalias[1:].encode('utf-8')) keyalias = m.hexdigest()[:8] else: - m = hashlib.md5() + m = hashlib.md5() # nosec just used to generate a keyalias m.update(appid.encode('utf-8')) keyalias = m.hexdigest()[:8] logging.info("Key alias: " + keyalias) diff --git a/fdroidserver/server.py b/fdroidserver/server.py index 0d6ffb32..5f1720b1 100644 --- a/fdroidserver/server.py +++ b/fdroidserver/server.py @@ -192,7 +192,7 @@ def update_awsbucket_libcloud(repo_section): upload = True else: # if the sizes match, then compare by MD5 - md5 = hashlib.md5() + md5 = hashlib.md5() # nosec AWS uses MD5 with open(file_to_upload, 'rb') as f: while True: data = f.read(8192) diff --git a/fdroidserver/update.py b/fdroidserver/update.py index bce32b21..22297b7e 100644 --- a/fdroidserver/update.py +++ b/fdroidserver/update.py @@ -27,7 +27,7 @@ import re import socket import zipfile import hashlib -import pickle +import pickle # nosec TODO import time import copy from datetime import datetime @@ -434,7 +434,7 @@ def getsig(apkpath): cert_encoded = common.get_certificate(cert) - return hashlib.md5(hexlify(cert_encoded)).hexdigest() + return hashlib.md5(hexlify(cert_encoded)).hexdigest() # nosec just used as ID for signing key def get_cache_file(): @@ -461,7 +461,7 @@ def get_cache(): ada = options.allow_disabled_algorithms or config['allow_disabled_algorithms'] if not options.clean and os.path.exists(apkcachefile): with open(apkcachefile, 'rb') as cf: - apkcache = pickle.load(cf, encoding='utf-8') + apkcache = pickle.load(cf, encoding='utf-8') # nosec TODO if apkcache.get("METADATA_VERSION") != METADATA_VERSION \ or apkcache.get('allow_disabled_algorithms') != ada: apkcache = {} @@ -1237,7 +1237,7 @@ def scan_apk_androguard(apk, apkfile): androidmanifest_xml = apkobject.xml['AndroidManifest.xml'] if len(xml.nsmap) > 0: # one of them surely will be the Android one, or its corrupt - xmlns = '{http://schemas.android.com/apk/res/android}' + xmlns = common.XMLNS_ANDROID else: # strange but sometimes the namespace is blank. This seems to # only happen with the Bromite/Chromium APKs diff --git a/setup.py b/setup.py index 4774d9d1..7a40a4d9 100755 --- a/setup.py +++ b/setup.py @@ -69,6 +69,7 @@ setup(name='fdroidserver', install_requires=[ 'androguard >= 3.1.0rc2', 'clint', + 'defusedxml', 'GitPython', 'mwclient', 'paramiko', diff --git a/tests/common.TestCase b/tests/common.TestCase index c2bde1a6..1c75a411 100755 --- a/tests/common.TestCase +++ b/tests/common.TestCase @@ -681,6 +681,12 @@ class CommonTest(unittest.TestCase): sig = fdroidserver.common.metadata_find_developer_signature('org.smssecure.smssecure') self.assertEqual('b30bb971af0d134866e158ec748fcd553df97c150f58b0a963190bbafbeb0868', sig) + def test_parse_xml(self): + manifest = os.path.join('source-files', 'fdroid', 'fdroidclient', 'AndroidManifest.xml') + parsed = fdroidserver.common.parse_xml(manifest) + self.assertIsNotNone(parsed) + self.assertEqual(str(type(parsed)), "") + def test_parse_androidmanifests(self): app = fdroidserver.metadata.App() app.id = 'org.fdroid.fdroid' @@ -800,13 +806,26 @@ class CommonTest(unittest.TestCase): fdroidserver.common.parse_androidmanifests(paths, app)) def test_calculate_math_string(self): - self.assertEqual(1234, fdroidserver.common.calculate_math_string('1234')) - self.assertEqual(4, fdroidserver.common.calculate_math_string('(1+1)*2')) - self.assertEqual(2, fdroidserver.common.calculate_math_string('(1-1)*2+3*1-1')) + self.assertEqual(1234, + fdroidserver.common.calculate_math_string('1234')) + self.assertEqual((1 + 1) * 2, + fdroidserver.common.calculate_math_string('(1 + 1) * 2')) + self.assertEqual((1 - 1) * 2 + 3 * 1 - 1, + fdroidserver.common.calculate_math_string('(1 - 1) * 2 + 3 * 1 - 1')) + self.assertEqual(0 - 12345, + fdroidserver.common.calculate_math_string('0 - 12345')) + self.assertEqual(0xffff, + fdroidserver.common.calculate_math_string('0xffff')) + self.assertEqual(0xcafe * 123, + fdroidserver.common.calculate_math_string('0xcafe * 123')) + self.assertEqual(-1, + fdroidserver.common.calculate_math_string('-1')) with self.assertRaises(SyntaxError): fdroidserver.common.calculate_math_string('__import__("urllib")') with self.assertRaises(SyntaxError): fdroidserver.common.calculate_math_string('self') + with self.assertRaises(SyntaxError): + fdroidserver.common.calculate_math_string('Ox9()') with self.assertRaises(SyntaxError): fdroidserver.common.calculate_math_string('1+1; print(1)') with self.assertRaises(SyntaxError):