From 6d4831a6dc3ccfee668145c893ff6a25d188035f Mon Sep 17 00:00:00 2001 From: Tomas Kopecek Date: Jan 18 2021 10:06:35 +0000 Subject: web: input validation Fix for CVE-2020-15856 Fixes: https://pagure.io/koji/issue/2645 --- diff --git a/www/kojiweb/index.py b/www/kojiweb/index.py index efb403d..a85f18c 100644 --- a/www/kojiweb/index.py +++ b/www/kojiweb/index.py @@ -42,6 +42,34 @@ def _sortbyname(x): return x['name'] +# regexps for input checking +_VALID_SEARCH_CHARS = r"""a-zA-Z0-9""" +_VALID_SEARCH_SYMS = r""" @.,_/\()%+-~*?|[]^$""" +_VALID_SEARCH_RE = re.compile('^[' + _VALID_SEARCH_CHARS + re.escape(_VALID_SEARCH_SYMS) + ']+$') + +_VALID_ARCH_RE = re.compile(r'^[\w-]+$', re.ASCII) + + +def _validate_arch(arch): + # archs (ASCII alnum + _ + -) + if not arch: + return None + elif _VALID_ARCH_RE.match(arch): + return arch + else: + raise koji.GenericError("Invalid arch: %r" % arch) + + +def _validate_name_or_id(value): + # integer ID or label, it is unicode alnum + search symbols (reasonable expectation?) + if value.isdigit(): + return int(value) + elif _VALID_SEARCH_RE.match(value): + return value + else: + raise koji.GenericError("Invalid int/label value: %r" % value) + + # loggers authlogger = logging.getLogger('koji.auth') @@ -496,10 +524,12 @@ def tasks(environ, owner=None, state='active', view='tree', method='all', hostID values = _initValues(environ, 'Tasks', 'tasks') server = _getServer(environ) + if view not in ('tree', 'toplevel', 'flat'): + raise koji.GenericError("Invalid value for view: %r" % view) + opts = {'decode': True} if owner: - if owner.isdigit(): - owner = int(owner) + owner = _validate_name_or_id(owner) ownerObj = server.getUser(owner, strict=True) opts['owner'] = ownerObj['id'] values['owner'] = ownerObj['name'] @@ -567,10 +597,7 @@ def tasks(environ, owner=None, state='active', view='tree', method='all', hostID values['hostID'] = None if channelID: - try: - channelID = int(channelID) - except ValueError: - pass + channelID = _validate_name_or_id(channelID) channel = server.getChannel(channelID, strict=True) opts['channel_id'] = channel['id'] values['channel'] = channel @@ -857,7 +884,10 @@ def tags(environ, start=None, order=None, childID=None): else: values['perms'] = [] - values['childID'] = childID + if childID is None: + values['childID'] = None + else: + values['childID'] = int(childID) return _genHTML(environ, 'tags.chtml') @@ -871,15 +901,13 @@ def packages(environ, tagID=None, userID=None, order='package_name', start=None, server = _getServer(environ) tag = None if tagID is not None: - if tagID.isdigit(): - tagID = int(tagID) + tagID = _validate_name_or_id(tagID) tag = server.getTag(tagID, strict=True) values['tagID'] = tagID values['tag'] = tag user = None if userID is not None: - if userID.isdigit(): - userID = int(userID) + userID = _validate_name_or_id(userID) user = server.getUser(userID, strict=True) values['userID'] = userID values['user'] = user @@ -909,8 +937,7 @@ def packageinfo(environ, packageID, tagOrder='name', tagStart=None, buildOrder=' values = _initValues(environ, 'Package Info', 'packages') server = _getServer(environ) - if packageID.isdigit(): - packageID = int(packageID) + packageID = _validate_name_or_id(packageID) package = server.getPackage(packageID) if package is None: raise koji.GenericError('invalid package ID: %s' % packageID) @@ -934,8 +961,7 @@ def taginfo(environ, tagID, all='0', packageOrder='package_name', packageStart=N values = _initValues(environ, 'Tag Info', 'tags') server = _getServer(environ) - if tagID.isdigit(): - tagID = int(tagID) + tagID = _validate_name_or_id(tagID) tag = server.getTag(tagID, strict=True) values['title'] = tag['name'] + ' | Tag Info' @@ -1149,8 +1175,7 @@ def externalrepoinfo(environ, extrepoID): values = _initValues(environ, 'External Repo Info', 'tags') server = _getServer(environ) - if extrepoID.isdigit(): - extrepoID = int(extrepoID) + extrepoID = _validate_name_or_id(extrepoID) extRepo = server.getExternalRepo(extrepoID, strict=True) repoTags = server.getTagExternalRepos(repo_info=extRepo['id']) @@ -1301,8 +1326,7 @@ def builds(environ, userID=None, tagID=None, packageID=None, state=None, order=' user = None if userID: - if userID.isdigit(): - userID = int(userID) + userID = _validate_name_or_id(userID) user = server.getUser(userID, strict=True) values['userID'] = userID values['user'] = user @@ -1314,16 +1338,14 @@ def builds(environ, userID=None, tagID=None, packageID=None, state=None, order=' tag = None if tagID: - if tagID.isdigit(): - tagID = int(tagID) + tagID = _validate_name_or_id(tagID) tag = server.getTag(tagID, strict=True) values['tagID'] = tagID values['tag'] = tag package = None if packageID: - if packageID.isdigit(): - packageID = int(packageID) + packageID = _validate_name_or_id(packageID) package = server.getPackage(packageID, strict=True) values['packageID'] = packageID values['package'] = package @@ -1409,8 +1431,7 @@ def userinfo(environ, userID, packageOrder='package_name', packageStart=None, values = _initValues(environ, 'User Info', 'users') server = _getServer(environ) - if userID.isdigit(): - userID = int(userID) + userID = _validate_name_or_id(userID) user = server.getUser(userID, strict=True) values['title'] = user['name'] + ' | User Info' @@ -1438,7 +1459,10 @@ def rpminfo(environ, rpmID, fileOrder='name', fileStart=None, buildrootOrder='-i server = _getServer(environ) rpmID = int(rpmID) - rpm = server.getRPM(rpmID) + try: + rpm = server.getRPM(rpmID, strict=True) + except koji.GenericError: + raise koji.GenericError('invalid RPM ID: %i' % rpmID) values['title'] = '%(name)s-%%s%(version)s-%(release)s.%(arch)s.rpm' % rpm + ' | RPM Info' epochStr = '' @@ -1621,8 +1645,7 @@ def hostinfo(environ, hostID=None, userID=None): server = _getServer(environ) if hostID: - if hostID.isdigit(): - hostID = int(hostID) + hostID = _validate_name_or_id(hostID) host = server.getHost(hostID) if host is None: raise koji.GenericError('invalid host ID: %s' % hostID) @@ -1757,8 +1780,7 @@ def channelinfo(environ, channelID): return _genHTML(environ, 'channelinfo.chtml') -def buildrootinfo(environ, buildrootID, builtStart=None, builtOrder=None, componentStart=None, - componentOrder=None): +def buildrootinfo(environ, buildrootID): values = _initValues(environ, 'Buildroot Info', 'hosts') server = _getServer(environ) @@ -2052,6 +2074,9 @@ def rpmsbyhost(environ, start=None, order=None, hostArch=None, rpmArch=None): values = _initValues(environ, 'RPMs by Host', 'reports') server = _getServer(environ) + hostArch = _validate_arch(hostArch) + rpmArch = _validate_arch(rpmArch) + maxRPMs = 1 hostArchFilter = hostArch if hostArchFilter == 'ix86': @@ -2126,6 +2151,7 @@ def tasksbyhost(environ, start=None, order='-tasks', hostArch=None): maxTasks = 1 + hostArch = _validate_arch(hostArch) hostArchFilter = hostArch if hostArchFilter == 'ix86': hostArchFilter = ['i386', 'i486', 'i586', 'i686'] @@ -2273,6 +2299,7 @@ def _filter_hosts_by_arch(hosts, arch): def clusterhealth(environ, arch='__all__'): + arch = _validate_arch(arch) values = _initValues(environ, 'Cluster health', 'reports') server = _getServer(environ) channels = server.listChannels() @@ -2329,21 +2356,18 @@ def recentbuilds(environ, user=None, tag=None, package=None): tagObj = None if tag is not None: - if tag.isdigit(): - tag = int(tag) - tagObj = server.getTag(tag) + tag = _validate_name_or_id(tag) + tagObj = server.getTag(tag, strict=True) userObj = None if user is not None: - if user.isdigit(): - user = int(user) - userObj = server.getUser(user) + user = _validate_name_or_id(user) + userObj = server.getUser(user, strict=True) packageObj = None if package: - if package.isdigit(): - package = int(package) - packageObj = server.getPackage(package) + package = _validate_name_or_id(package) + packageObj = server.getPackage(package, strict=True) if tagObj is not None: builds = server.listTagged(tagObj['id'], inherit=True, @@ -2404,9 +2428,6 @@ _infoURLs = {'package': 'packageinfo?packageID=%(id)i', 'maven': 'archiveinfo?archiveID=%(id)i', 'win': 'archiveinfo?archiveID=%(id)i'} -_VALID_SEARCH_CHARS = r"""a-zA-Z0-9""" -_VALID_SEARCH_SYMS = r""" @.,_/\()%+-~*?|[]^$""" -_VALID_SEARCH_RE = re.compile('^[' + _VALID_SEARCH_CHARS + re.escape(_VALID_SEARCH_SYMS) + ']+$') _DEFAULT_SEARCH_ORDER = { # For searches against large tables, use '-id' to show most recent first 'build': '-id', @@ -2422,6 +2443,8 @@ _DEFAULT_SEARCH_ORDER = { def search(environ, start=None, order=None): + if start is not None: + start = int(start) values = _initValues(environ, 'Search', 'search') server = _getServer(environ) values['error'] = None @@ -2436,10 +2459,14 @@ def search(environ, start=None, order=None): values['type'] = type values['match'] = match + if match not in ('glob', 'regexp', 'exact'): + raise koji.GenericError("Invalid match type: %r" % match) + if not _VALID_SEARCH_RE.match(terms): values['error'] = 'Invalid search terms
' + \ 'Search terms may contain only these characters: ' + \ _VALID_SEARCH_CHARS + _VALID_SEARCH_SYMS + values['terms'] = '' return _genHTML(environ, 'search.chtml') if match == 'regexp': @@ -2447,6 +2474,7 @@ def search(environ, start=None, order=None): re.compile(terms) except Exception: values['error'] = 'Invalid regular expression' + values['terms'] = '' return _genHTML(environ, 'search.chtml') infoURL = _infoURLs.get(type) @@ -2492,9 +2520,9 @@ def api(environ): def watchlogs(environ, taskID): values = _initValues(environ) if isinstance(taskID, list): - values['tasks'] = ', '.join(taskID) + values['tasks'] = ', '.join([int(x) for x in taskID]) else: - values['tasks'] = taskID + values['tasks'] = int(taskID) html = """