From 6cd8f2ffeaaf943a716dd9dee73eb5d3da6db754 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 2 Mar 2018 10:21:55 +0100 Subject: [PATCH 1/3] SVN: only allow redirects to HTTPS "SVN follows HTTP 301 redirects to svn+ssh:// URLs. As a result, an innocent looking HTTP URL can be used to trigger a Command Execution with a 301 redirect." https://blog.recurity-labs.com/2017-08-10/scm-vulns.html#third-round-svn-and-mercurial I scanned fdroiddata and found no suspicious redirects. Here's how: grep -A1 '^Repo *Type: *git-svn' *.txt *.yml| sed -n 's,.*Repo:\(.*\),\1,p' > /tmp/urls.txt import requests with open('/tmp/urls.txt') as fp: for line in fp: try: r = requests.head(line.strip()) print(r.status_code, line) except requests.exceptions.SSLError: print('SSLError', line) --- fdroidserver/common.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fdroidserver/common.py b/fdroidserver/common.py index 83dfb441..b943fc50 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -1011,6 +1011,10 @@ class vcs_gitsvn(vcs): import requests r = requests.head(remote) r.raise_for_status() + location = r.headers.get('location') + if location and not location.startswith('https://'): + raise VCSException(_('Invalid redirect to non-HTTPS: {before} -> {after} ') + .format(before=remote, after=location)) gitsvn_args.extend(['--', remote, self.local]) p = self.git(gitsvn_args) From 6876e28bb41f16b676628d7274da6c4b1b799a41 Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 2 Mar 2018 11:06:26 +0100 Subject: [PATCH 2/3] hg: use /bin/false to clarify that it is an executable --- fdroidserver/common.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fdroidserver/common.py b/fdroidserver/common.py index b943fc50..93a6945f 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -1116,7 +1116,7 @@ class vcs_hg(vcs): def gotorevisionx(self, rev): if not os.path.exists(self.local): - p = FDroidPopen(['hg', 'clone', '--ssh', 'false', '--', self.remote, self.local], + p = FDroidPopen(['hg', 'clone', '--ssh', '/bin/false', '--', self.remote, self.local], output=False) if p.returncode != 0: self.clone_failed = True @@ -1130,7 +1130,7 @@ class vcs_hg(vcs): raise VCSException("Unexpected output from hg status -uS: " + line) FDroidPopen(['rm', '-rf', '--', line[2:]], cwd=self.local, output=False) if not self.refreshed: - p = FDroidPopen(['hg', 'pull', '--ssh', 'false'], cwd=self.local, output=False) + p = FDroidPopen(['hg', 'pull', '--ssh', '/bin/false'], cwd=self.local, output=False) if p.returncode != 0: raise VCSException("Hg pull failed", p.output) self.refreshed = True From 8f30c892c5432b93875eb08699ac966663c46dcc Mon Sep 17 00:00:00 2001 From: Hans-Christoph Steiner Date: Fri, 2 Mar 2018 12:50:48 +0100 Subject: [PATCH 3/3] VercodeOperation: only allow simple math expresssions and %c --- fdroidserver/checkupdates.py | 3 +++ fdroidserver/common.py | 2 ++ fdroidserver/lint.py | 6 +++++ tests/lint.TestCase | 47 ++++++++++++++++++++++++++++++++++++ 4 files changed, 58 insertions(+) diff --git a/fdroidserver/checkupdates.py b/fdroidserver/checkupdates.py index d919c72b..0a4f6e27 100644 --- a/fdroidserver/checkupdates.py +++ b/fdroidserver/checkupdates.py @@ -429,6 +429,9 @@ def checkupdates_app(app): msg = 'Invalid update check method' if version and vercode and app.VercodeOperation: + if not common.VERCODE_OPERATION_RE.match(app.VercodeOperation): + raise MetaDataException(_('Invalid VercodeOperation: {field}') + .format(field=app.VercodeOperation)) oldvercode = str(int(vercode)) op = app.VercodeOperation.replace("%c", oldvercode) vercode = str(eval(op)) diff --git a/fdroidserver/common.py b/fdroidserver/common.py index 93a6945f..cf1d9203 100644 --- a/fdroidserver/common.py +++ b/fdroidserver/common.py @@ -61,6 +61,8 @@ from .asynchronousfilereader import AsynchronousFileReader # has to be manually set in test_aapt_version() MINIMUM_AAPT_VERSION = '26.0.0' +VERCODE_OPERATION_RE = re.compile(r'^([ 0-9/*+-]|%c)+$') + # A signature block file with a .DSA, .RSA, or .EC extension 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') diff --git a/fdroidserver/lint.py b/fdroidserver/lint.py index ed86e29a..a40ffffe 100644 --- a/fdroidserver/lint.py +++ b/fdroidserver/lint.py @@ -222,6 +222,11 @@ def check_update_check_data_url(app): yield _('UpdateCheckData must use HTTPS URL: {url}').format(url=url) +def check_vercode_operation(app): + if app.VercodeOperation and not common.VERCODE_OPERATION_RE.match(app.VercodeOperation): + yield _('Invalid VercodeOperation: {field}').format(field=app.VercodeOperation) + + def check_ucm_tags(app): lastbuild = get_lastbuild(app.builds) if (lastbuild is not None @@ -529,6 +534,7 @@ def main(): app_check_funcs = [ check_regexes, check_update_check_data_url, + check_vercode_operation, check_ucm_tags, check_char_limits, check_old_links, diff --git a/tests/lint.TestCase b/tests/lint.TestCase index ad9f0514..28d539a0 100755 --- a/tests/lint.TestCase +++ b/tests/lint.TestCase @@ -19,6 +19,7 @@ if localmodule not in sys.path: import fdroidserver.common import fdroidserver.lint +import fdroidserver.metadata class LintTest(unittest.TestCase): @@ -69,6 +70,52 @@ class LintTest(unittest.TestCase): logging.debug(warn) self.assertTrue(anywarns) + def test_check_vercode_operation(self): + config = dict() + fdroidserver.common.fill_config_defaults(config) + fdroidserver.common.config = config + fdroidserver.lint.config = config + + app = fdroidserver.metadata.App() + app.Name = 'Bad App' + app.Summary = 'We pwn you' + app.Description = 'These are some back' + + good_fields = [ + '6%c', + '%c - 1', + '%c + 10', + '%c*10', + '%c*10 + 3', + '%c*10 + 8', + '%c + 2 ', + '%c + 3', + '%c + 7', + ] + bad_fields = [ + 'open("/etc/passwd")', + '%C + 1', + '%%c * 123', + '123 + %%', + '%c % 7', + ] + + anywarns = False + for good in good_fields: + app.VercodeOperation = good + for warn in fdroidserver.lint.check_vercode_operation(app): + anywarns = True + logging.debug(warn) + self.assertFalse(anywarns) + + for bad in bad_fields: + anywarns = False + app.VercodeOperation = bad + for warn in fdroidserver.lint.check_vercode_operation(app): + anywarns = True + logging.debug(warn) + self.assertTrue(anywarns) + if __name__ == "__main__": parser = optparse.OptionParser()