Merge branch 'minor-security-ish-tweaks' into 'master'

security-ish tweaks

See merge request fdroid/fdroidserver!442

buildserver/provision-apt-get-install

@@ -89,10 +89,13 @@ packages="
  python-lxml
  python-magic
  python-setuptools
+ python3-git/jessie-backports
+ python3-gitdb/jessie-backports
  python3-gnupg
  python3-pyasn1
  python3-pyasn1-modules
  python3-requests
+ python3-smmap/jessie-backports
  python3-yaml
  python3-ruamel.yaml
  quilt

fdroidserver/build.py

@@ -133,9 +133,9 @@ def build_server(app, build, vcs, build_dir, output_dir, log_dir, force):
         ftp.chmod('config.py', 0o600)
 
         # Copy over the ID (head commit hash) of the fdroidserver in use...
-        subprocess.call('git rev-parse HEAD >' +
+        with open(os.path.join(os.getcwd(), 'tmp', 'fdroidserverid'), 'wb') as fp:
-                        os.path.join(os.getcwd(), 'tmp', 'fdroidserverid'),
+            fp.write(subprocess.check_output(['git', 'rev-parse', 'HEAD'],
-                        shell=True, cwd=serverpath)
+                                             cwd=serverpath))
         ftp.put('tmp/fdroidserverid', 'fdroidserverid')
 
         # Copy the metadata - just the file for this app...
@@ -1263,7 +1263,7 @@ def main():
             for app in build_succeeded:
 
                 logging.info("Need to sign the app before we can install it.")
-                subprocess.call("fdroid publish {0}".format(app.id), shell=True)
+                subprocess.call("fdroid publish {0}".format(app.id))
 
                 apk_path = None
 

fdroidserver/common.py

@@ -849,7 +849,7 @@ class vcs_git(vcs):
     def gotorevisionx(self, rev):
         if not os.path.exists(self.local):
             # Brand new checkout
-            p = self.git(['clone', self.remote, self.local])
+            p = self.git(['clone', '--', self.remote, self.local])
             if p.returncode != 0:
                 self.clone_failed = True
                 raise VCSException("Git clone failed", p.output)
@@ -882,7 +882,8 @@ class vcs_git(vcs):
                     if 'Multiple remote HEAD branches' not in lines[0]:
                         raise VCSException(_("Git remote set-head failed"), p.output)
                     branch = lines[1].split(' ')[-1]
-                    p2 = FDroidPopen(['git', 'remote', 'set-head', 'origin', branch], cwd=self.local, output=False)
+                    p2 = FDroidPopen(['git', 'remote', 'set-head', 'origin', '--', branch],
+                                     cwd=self.local, output=False)
                     if p2.returncode != 0:
                         raise VCSException(_("Git remote set-head failed"), p.output + '\n' + p2.output)
                 self.refreshed = True
@@ -1090,7 +1091,8 @@ 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], output=False)
+            p = FDroidPopen(['hg', 'clone', '--ssh', 'false', '--', self.remote, self.local],
+                            output=False)
             if p.returncode != 0:
                 self.clone_failed = True
                 raise VCSException("Hg clone failed", p.output)
@@ -1101,7 +1103,7 @@ class vcs_hg(vcs):
             for line in p.output.splitlines():
                 if not line.startswith('? '):
                     raise VCSException("Unexpected output from hg status -uS: " + line)
-                FDroidPopen(['rm', '-rf', line[2:]], cwd=self.local, output=False)
+                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)
                 if p.returncode != 0:
@@ -1111,7 +1113,7 @@ class vcs_hg(vcs):
         rev = rev or 'default'
         if not rev:
             return
-        p = FDroidPopen(['hg', 'update', '-C', rev], cwd=self.local, output=False)
+        p = FDroidPopen(['hg', 'update', '-C', '--', rev], cwd=self.local, output=False)
         if p.returncode != 0:
             raise VCSException("Hg checkout of '%s' failed" % rev, p.output)
         p = FDroidPopen(['hg', 'purge', '--all'], cwd=self.local, output=False)
@@ -1511,7 +1513,7 @@ def getsrclib(spec, srclib_dir, subdir=None, basepath=False,
         if srclib["Prepare"]:
             cmd = replace_config_vars(srclib["Prepare"], build)
 
-            p = FDroidPopen(['bash', '-x', '-c', cmd], cwd=libdir)
+            p = FDroidPopen(['bash', '-x', '-c', '--', cmd], cwd=libdir)
             if p.returncode != 0:
                 raise BuildException("Error running prepare command for srclib %s"
                                      % name, p.output)
@@ -1566,7 +1568,7 @@ def prepare_source(vcs, app, build, build_dir, srclib_dir, extlib_dir, onserver=
         cmd = replace_config_vars(build.init, build)
         logging.info("Running 'init' commands in %s" % root_dir)
 
-        p = FDroidPopen(['bash', '-x', '-c', cmd], cwd=root_dir)
+        p = FDroidPopen(['bash', '-x', '-c', '--', cmd], cwd=root_dir)
         if p.returncode != 0:
             raise BuildException("Error running init command for %s:%s" %
                                  (app.id, build.versionName), p.output)
@@ -1724,7 +1726,7 @@ def prepare_source(vcs, app, build, build_dir, srclib_dir, extlib_dir, onserver=
             libpath = os.path.relpath(libpath, root_dir)
             cmd = cmd.replace('$$' + name + '$$', libpath)
 
-        p = FDroidPopen(['bash', '-x', '-c', cmd], cwd=root_dir)
+        p = FDroidPopen(['bash', '-x', '-c', '--', cmd], cwd=root_dir)
         if p.returncode != 0:
             raise BuildException("Error running prebuild command for %s:%s" %
                                  (app.id, build.versionName), p.output)

fdroidserver/lint.py

@@ -164,6 +164,10 @@ regex_checks = {
          _("Unnecessary leading space")),
         (re.compile(r'.*\s$'),
          _("Unnecessary trailing space")),
+        (re.compile(r'.*<(iframe|link|script).*'),
+         _("Forbidden HTML tags")),
+        (re.compile(r'''.*\s+src=["']javascript:.*'''),
+         _("Javascript in HTML src attributes")),
     ],
 }
 

fdroidserver/scanner.py

@@ -40,7 +40,7 @@ def get_gradle_compile_commands(build):
     return [re.compile(r'\s*' + c, re.IGNORECASE) for c in compileCommands]
 
 
-def scan_source(build_dir, build):
+def scan_source(build_dir, build=metadata.Build()):
     """Scan the source code in the given directory (and all subdirectories)
     and return the number of fatal problems encountered
     """
@@ -294,19 +294,25 @@ def main():
         if app.Disabled:
             logging.info(_("Skipping {appid}: disabled").format(appid=appid))
             continue
-        if not app.builds:
-            logging.info(_("Skipping {appid}: no builds specified").format(appid=appid))
-            continue
-
-        logging.info(_("Processing {appid}").format(appid=appid))
 
         try:
-
             if app.RepoType == 'srclib':
                 build_dir = os.path.join('build', 'srclib', app.Repo)
             else:
                 build_dir = os.path.join('build', appid)
 
+            if app.builds:
+                logging.info(_("Processing {appid}").format(appid=appid))
+            else:
+                logging.info(_("{appid}: no builds specified, running on current source state")
+                             .format(appid=appid))
+                count = scan_source(build_dir)
+                if count > 0:
+                    logging.warn(_('Scanner found {count} problems in {appid}:')
+                                 .format(count=count, appid=appid))
+                    probcount += count
+                continue
+
             # Set up vcs interface and make sure we have the latest code...
             vcs = common.getvcs(app.RepoType, app.Repo, build_dir)
 
@@ -315,20 +321,19 @@ def main():
                 if build.disable:
                     logging.info("...skipping version %s - %s" % (
                         build.versionName, build.get('disable', build.commit[1:])))
-                else:
+                    continue
-                    logging.info("...scanning version " + build.versionName)
 
-                    # Prepare the source code...
+                logging.info("...scanning version " + build.versionName)
-                    common.prepare_source(vcs, app, build,
+                # Prepare the source code...
-                                          build_dir, srclib_dir,
+                common.prepare_source(vcs, app, build,
-                                          extlib_dir, False)
+                                      build_dir, srclib_dir,
+                                      extlib_dir, False)
 
-                    # Do the scan...
+                count = scan_source(build_dir, build)
-                    count = scan_source(build_dir, build)
+                if count > 0:
-                    if count > 0:
+                    logging.warn(_('Scanner found {count} problems in {appid}:{versionCode}:')
-                        logging.warn('Scanner found %d problems in %s (%s)' % (
+                                 .format(count=count, appid=appid, versionCode=build.versionCode))
-                            count, appid, build.versionCode))
+                    probcount += count
-                        probcount += count
 
         except BuildException as be:
             logging.warn("Could not scan app %s due to BuildException: %s" % (

fdroidserver/update.py

@@ -140,7 +140,7 @@ def update_wiki(apps, sortedids, apks):
             requiresroot = 'Yes'
         else:
             requiresroot = 'No'
-        wikidata += '{{App|id=%s|name=%s|added=%s|lastupdated=%s|source=%s|tracker=%s|web=%s|changelog=%s|donate=%s|flattr=%s|liberapay=%s|bitcoin=%s|litecoin=%s|license=%s|root=%s|author=%s|email=%s}}\n' % (
+        wikidata += '{{App|id=%s|name=%s|added=%s|lastupdated=%s|source=%s|tracker=%s|web=%s|changelog=%s|donate=%s|flattr=%s|liberapay=%s|bitcoin=%s|litecoin=%s|license=%s|root=%s|author=%s|email=%s|activity=%s}}\n' % (
             appid,
             app.Name,
             app.added.strftime('%Y-%m-%d') if app.added else '',
@@ -157,7 +157,9 @@ def update_wiki(apps, sortedids, apks):
             app.License,
             requiresroot,
             app.AuthorName,
-            app.AuthorEmail)
+            app.AuthorEmail,
+            'https://gitlab.com/search?group_id=28397&scope=issues&search=' + appid,
+        )
 
         if app.Provides:
             wikidata += "This app provides: %s" % ', '.join(app.Summary.split(','))

fdroidserver/vmtools.py

@@ -88,14 +88,14 @@ def get_clean_builder(serverdir, reset=False):
     return sshinfo
 
 
-def _check_call(cmd, shell=False, cwd=None):
+def _check_call(cmd, cwd=None):
     logger.debug(' '.join(cmd))
-    return subprocess.check_call(cmd, shell=shell, cwd=cwd)
+    return subprocess.check_call(cmd, shell=False, cwd=cwd)
 
 
-def _check_output(cmd, shell=False, cwd=None):
+def _check_output(cmd, cwd=None):
     logger.debug(' '.join(cmd))
-    return subprocess.check_output(cmd, shell=shell, cwd=cwd)
+    return subprocess.check_output(cmd, shell=False, cwd=cwd)
 
 
 def get_build_vm(srvdir, provider=None):
@@ -303,11 +303,13 @@ class FDroidBuildVm():
         """
         import paramiko
         try:
-            _check_call(['vagrant ssh-config > sshconfig'],
+            sshconfig_path = os.path.join(self.srvdir, 'sshconfig')
-                        cwd=self.srvdir, shell=True)
+            with open(sshconfig_path, 'wb') as fp:
+                fp.write(_check_output(['vagrant', 'ssh-config'],
+                                       cwd=self.srvdir))
             vagranthost = 'default'  # Host in ssh config file
             sshconfig = paramiko.SSHConfig()
-            with open(joinpath(self.srvdir, 'sshconfig'), 'r') as f:
+            with open(sshconfig_path, 'r') as f:
                 sshconfig.parse(f)
             sshconfig = sshconfig.lookup(vagranthost)
             idfile = sshconfig['identityfile']

hooks/pre-commit

@@ -129,4 +129,8 @@ for f in $RB_FILES; do
 	fi
 done
 
+if grep --line-number 'shell=True' fdroidserver/[a-ce-z]*.py; then
+    err "shell=True is too dangerous, there are unfiltered user inputs!"
+fi
+
 exit 0

tests/check-fdroid-apk

@@ -35,8 +35,8 @@ apk="${dldir}/FDroid.apk"
 asc="${apk}.asc"
 log=/var/www/html/check-fdroid-apk/`date +%s`.txt
 
-$curl https://f-droid.org/FDroid.apk > $apk
+$curl --user-agent F-Droid https://f-droid.org/FDroid.apk > $apk
-$curl https://f-droid.org/FDroid.apk.asc > $asc
+$curl --user-agent F-Droid https://f-droid.org/FDroid.apk.asc > $asc
 
 fingerprint=37D2C98789D8311948394E3E41E7044E1DBA2E89