# HG changeset patch # User Marcin Lulek # Date 2017-06-26 12:48:15 # Node ID b8e3feed21dbae501c8581655c16df6e90425eb1 # Parent a499b95c484c2501e86decd6eeafc4ddd93568cc security: escape flash messaged VCS errors to prevent XSS atacks. diff --git a/rhodecode/controllers/changelog.py b/rhodecode/controllers/changelog.py --- a/rhodecode/controllers/changelog.py +++ b/rhodecode/controllers/changelog.py @@ -67,13 +67,11 @@ class ChangelogController(BaseRepoContro 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(BaseRepoContro 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(BaseRepoContro 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') diff --git a/rhodecode/controllers/compare.py b/rhodecode/controllers/compare.py --- a/rhodecode/controllers/compare.py +++ b/rhodecode/controllers/compare.py @@ -24,7 +24,7 @@ Compare controller for showing differenc 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(BaseRepoControll 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(BaseRepoControll 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(BaseRepoControll 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() diff --git a/rhodecode/controllers/files.py b/rhodecode/controllers/files.py --- a/rhodecode/controllers/files.py +++ b/rhodecode/controllers/files.py @@ -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')) diff --git a/rhodecode/lib/vcs/backends/base.py b/rhodecode/lib/vcs/backends/base.py --- a/rhodecode/lib/vcs/backends/base.py +++ b/rhodecode/lib/vcs/backends/base.py @@ -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): """ diff --git a/rhodecode/tests/functional/test_compare.py b/rhodecode/tests/functional/test_compare.py --- a/rhodecode/tests/functional/test_compare.py +++ b/rhodecode/tests/functional/test_compare.py @@ -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) diff --git a/rhodecode/tests/functional/test_files.py b/rhodecode/tests/functional/test_files.py --- a/rhodecode/tests/functional/test_files.py +++ b/rhodecode/tests/functional/test_files.py @@ -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):