security: escape flash messaged VCS errors to prevent XSS atacks.

This commit is contained in:
Marcin Lulek 2017-06-26 14:48:15 +02:00
parent 5fc0ee97f9
commit cb6d81f435
6 changed files with 48 additions and 34 deletions

View file

@ -67,13 +67,11 @@ class ChangelogController(BaseRepoController):
except EmptyRepositoryError:
if not redirect_after:
return None
h.flash(h.literal(_('There are no commits yet')),
category='warning')
h.flash(_('There are no commits yet'), category='warning')
redirect(url('changelog_home', repo_name=repo.repo_name))
except RepositoryError as e:
msg = safe_str(e)
log.exception(msg)
h.flash(msg, category='warning')
log.exception(safe_str(e))
h.flash(safe_str(h.escape(e)), category='warning')
if not partial:
redirect(h.url('changelog_home', repo_name=repo.repo_name))
raise HTTPBadRequest()
@ -113,7 +111,7 @@ class ChangelogController(BaseRepoController):
def _check_if_valid_branch(self, branch_name, repo_name, f_path):
if branch_name not in c.rhodecode_repo.branches_all:
h.flash('Branch {} is not found.'.format(branch_name),
h.flash('Branch {} is not found.'.format(h.escape(branch_name)),
category='warning')
redirect(url('changelog_file_home', repo_name=repo_name,
revision=branch_name, f_path=f_path or ''))
@ -189,12 +187,11 @@ class ChangelogController(BaseRepoController):
collection, p, chunk_size, c.branch_name, dynamic=f_path)
except EmptyRepositoryError as e:
h.flash(safe_str(e), category='warning')
h.flash(safe_str(h.escape(e)), category='warning')
return redirect(h.route_path('repo_summary', repo_name=repo_name))
except (RepositoryError, CommitDoesNotExistError, Exception) as e:
msg = safe_str(e)
log.exception(msg)
h.flash(msg, category='error')
log.exception(safe_str(e))
h.flash(safe_str(h.escape(e)), category='error')
return redirect(url('changelog_home', repo_name=repo_name))
if (request.environ.get('HTTP_X_PARTIAL_XHR')

View file

@ -24,7 +24,7 @@ Compare controller for showing differences between two commits/refs/tags etc.
import logging
from webob.exc import HTTPBadRequest
from webob.exc import HTTPBadRequest, HTTPNotFound
from pylons import request, tmpl_context as c, url
from pylons.controllers.util import redirect
from pylons.i18n.translation import _
@ -66,9 +66,8 @@ class CompareController(BaseRepoController):
redirect(h.route_path('repo_summary', repo_name=repo.repo_name))
except RepositoryError as e:
msg = safe_str(e)
log.exception(msg)
h.flash(msg, category='warning')
log.exception(safe_str(e))
h.flash(safe_str(h.escape(e)), category='warning')
if not partial:
redirect(h.route_path('repo_summary', repo_name=repo.repo_name))
raise HTTPBadRequest()
@ -86,6 +85,10 @@ class CompareController(BaseRepoController):
target_repo = request.GET.get('target_repo', source_repo)
c.source_repo = Repository.get_by_repo_name(source_repo)
c.target_repo = Repository.get_by_repo_name(target_repo)
if c.source_repo is None or c.target_repo is None:
raise HTTPNotFound()
c.source_ref = c.target_ref = _('Select commit')
c.source_ref_type = ""
c.target_ref_type = ""
@ -141,18 +144,17 @@ class CompareController(BaseRepoController):
target_repo = Repository.get_by_repo_name(target_repo_name)
if source_repo is None:
msg = _('Could not find the original repo: %(repo)s') % {
'repo': source_repo}
log.error(msg)
h.flash(msg, category='error')
log.error('Could not find the source repo: {}'
.format(source_repo_name))
h.flash(_('Could not find the source repo: `{}`')
.format(h.escape(source_repo_name)), category='error')
return redirect(url('compare_home', repo_name=c.repo_name))
if target_repo is None:
msg = _('Could not find the other repo: %(repo)s') % {
'repo': target_repo_name}
log.error(msg)
h.flash(msg, category='error')
log.error('Could not find the target repo: {}'
.format(source_repo_name))
h.flash(_('Could not find the target repo: `{}`')
.format(h.escape(target_repo_name)), category='error')
return redirect(url('compare_home', repo_name=c.repo_name))
source_scm = source_repo.scm_instance()

View file

@ -107,7 +107,7 @@ class FilesController(BaseRepoController):
h.flash(msg, category='error')
raise HTTPNotFound()
except RepositoryError as e:
h.flash(safe_str(e), category='error')
h.flash(safe_str(h.escape(e)), category='error')
raise HTTPNotFound()
def __get_filenode_or_redirect(self, repo_name, commit, path):
@ -128,7 +128,7 @@ class FilesController(BaseRepoController):
h.flash(_('No such commit exists for this repository'), category='error')
raise HTTPNotFound()
except RepositoryError as e:
h.flash(safe_str(e), category='error')
h.flash(safe_str(h.escape(e)), category='error')
raise HTTPNotFound()
return file_node
@ -256,7 +256,7 @@ class FilesController(BaseRepoController):
repo_name, c.commit.raw_id, f_path)
except RepositoryError as e:
h.flash(safe_str(e), category='error')
h.flash(safe_str(h.escape(e)), category='error')
raise HTTPNotFound()
if request.environ.get('HTTP_X_PJAX'):
@ -472,9 +472,8 @@ class FilesController(BaseRepoController):
_('Successfully deleted file `{}`').format(
h.escape(f_path)), category='success')
except Exception:
msg = _('Error occurred during commit')
log.exception(msg)
h.flash(msg, category='error')
log.exception('Error during commit operation')
h.flash(_('Error occurred during commit'), category='error')
return redirect(url('changeset_home',
repo_name=c.repo_name, revision='tip'))

View file

@ -1066,7 +1066,7 @@ class BaseCommit(object):
def no_node_at_path(self, path):
return NodeDoesNotExistError(
u"There is no file nor directory at the given path: "
u"'%s' at commit %s" % (safe_unicode(path), self.short_id))
u"`%s` at commit %s" % (safe_unicode(path), self.short_id))
def _fix_path(self, path):
"""

View file

@ -489,7 +489,22 @@ class TestCompareController(object):
compare_page = ComparePage(response)
compare_page.contains_commits(commits=[commit1], ancestors=[commit0])
def test_errors_when_comparing_unknown_repo(self, backend):
def test_errors_when_comparing_unknown_source_repo(self, backend):
repo = backend.repo
badrepo = 'badrepo'
response = self.app.get(
url('compare_url',
repo_name=badrepo,
source_ref_type="rev",
source_ref='tip',
target_ref_type="rev",
target_ref='tip',
target_repo=repo.repo_name,
merge='1',),
status=404)
def test_errors_when_comparing_unknown_target_repo(self, backend):
repo = backend.repo
badrepo = 'badrepo'
@ -504,7 +519,8 @@ class TestCompareController(object):
merge='1',),
status=302)
redirected = response.follow()
redirected.mustcontain('Could not find the other repo: %s' % badrepo)
redirected.mustcontain(
'Could not find the target repo: `{}`'.format(badrepo))
def test_compare_not_in_preview_mode(self, backend_stub):
commit0 = backend_stub.repo.get_commit(commit_idx=0)

View file

@ -484,7 +484,7 @@ class TestRawFileHandling(object):
msg = (
"There is no file nor directory at the given path: "
"'%s' at commit %s" % (f_path, commit.short_id))
"`%s` at commit %s" % (f_path, commit.short_id))
response.mustcontain(msg)
def test_raw_ok(self, backend):
@ -517,7 +517,7 @@ class TestRawFileHandling(object):
f_path=f_path), status=404)
msg = (
"There is no file nor directory at the given path: "
"'%s' at commit %s" % (f_path, commit.short_id))
"`%s` at commit %s" % (f_path, commit.short_id))
response.mustcontain(msg)
def test_raw_svg_should_not_be_rendered(self, backend):