web: input validation
Fix for CVE-2020-15856 Fixes: https://pagure.io/koji/issue/2645
This commit is contained in:
Tomas Kopecek
parent
a5edca87e6
commit
6d4831a6dc
2 changed files with 83 additions and 44 deletions
|
|
@ -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<br/>' + \
|
||||
'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 = """
|
||||
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
|
||||
|
|
@ -2518,6 +2546,7 @@ def repoinfo(environ, repoID):
|
|||
values = _initValues(environ, 'Repo Info', 'tags')
|
||||
server = _getServer(environ)
|
||||
|
||||
repoID = _validate_name_or_id(repoID)
|
||||
values['repo_id'] = repoID
|
||||
repo_info = server.repoInfo(repoID, strict=False)
|
||||
values['repo'] = repo_info
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@
|
|||
import datetime
|
||||
import hashlib
|
||||
import os
|
||||
import re
|
||||
import ssl
|
||||
import stat
|
||||
import xmlrpc.client
|
||||
|
|
@ -49,6 +50,9 @@ except Exception:
|
|||
themeInfo = {}
|
||||
themeCache = {}
|
||||
|
||||
# allowed values for SQL ordering (e.g. -id, package_name, etc.)
|
||||
RE_ORDER = re.compile(r'^-?\w+$')
|
||||
|
||||
|
||||
def _initValues(environ, title='Build System Info', pageID='summary'):
|
||||
global themeInfo
|
||||
|
|
@ -290,6 +294,8 @@ def paginateList(values, data, start, dataName, prefix=None, order=None, noneGre
|
|||
be added to the value map.
|
||||
"""
|
||||
if order is not None:
|
||||
if not RE_ORDER.match(order):
|
||||
raise ValueError("Ordering is not alphanumeric: %r" % order)
|
||||
if order.startswith('-'):
|
||||
order = order[1:]
|
||||
reverse = True
|
||||
|
|
@ -327,6 +333,8 @@ def paginateMethod(server, values, methodName, args=None, kw=None,
|
|||
start = 0
|
||||
if not dataName:
|
||||
raise Exception('dataName must be specified')
|
||||
if not RE_ORDER.match(order):
|
||||
raise ValueError("Ordering is not alphanumeric: %r" % order)
|
||||
|
||||
kw['queryOpts'] = {'countOnly': True}
|
||||
totalRows = getattr(server, methodName)(*args, **kw)
|
||||
|
|
@ -358,6 +366,8 @@ def paginateResults(server, values, methodName, args=None, kw=None,
|
|||
start = 0
|
||||
if not dataName:
|
||||
raise Exception('dataName must be specified')
|
||||
if not RE_ORDER.match(order):
|
||||
raise ValueError("Ordering is not alphanumeric: %r" % order)
|
||||
|
||||
kw['filterOpts'] = {'order': order,
|
||||
'offset': start,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue