diff --git a/rhodecode/apps/admin/tests/test_admin_settings.py b/rhodecode/apps/admin/tests/test_admin_settings.py --- a/rhodecode/apps/admin/tests/test_admin_settings.py +++ b/rhodecode/apps/admin/tests/test_admin_settings.py @@ -620,11 +620,11 @@ class TestAdminSettingsIssueTracker(obje post_url = route_path('admin_settings_issuetracker_update') post_data = { 'new_pattern_pattern_0': pattern, - 'new_pattern_url_0': 'url', + 'new_pattern_url_0': 'http://url', 'new_pattern_prefix_0': 'prefix', 'new_pattern_description_0': 'description', 'new_pattern_pattern_1': another_pattern, - 'new_pattern_url_1': 'url1', + 'new_pattern_url_1': 'https://url1', 'new_pattern_prefix_1': 'prefix1', 'new_pattern_description_1': 'description1', 'csrf_token': csrf_token @@ -663,7 +663,7 @@ class TestAdminSettingsIssueTracker(obje post_url = route_path('admin_settings_issuetracker_update') post_data = { 'new_pattern_pattern_0': pattern, - 'new_pattern_url_0': 'url', + 'new_pattern_url_0': 'https://url', 'new_pattern_prefix_0': 'prefix', 'new_pattern_description_0': 'description', 'uid': old_uid, @@ -697,7 +697,7 @@ class TestAdminSettingsIssueTracker(obje post_url = route_path('admin_settings_issuetracker_update') post_data = { 'new_pattern_pattern_0': pattern, - 'new_pattern_url_0': 'url', + 'new_pattern_url_0': 'https://url', 'new_pattern_prefix_0': 'prefix', 'new_pattern_description_0': new_description, 'uid': self.uid, diff --git a/rhodecode/apps/admin/views/settings.py b/rhodecode/apps/admin/views/settings.py --- a/rhodecode/apps/admin/views/settings.py +++ b/rhodecode/apps/admin/views/settings.py @@ -481,7 +481,15 @@ class AdminSettingsView(BaseAppView): self.load_default_context() settings_model = IssueTrackerSettingsModel() - form = IssueTrackerPatternsForm()().to_python(self.request.POST) + try: + form = IssueTrackerPatternsForm()().to_python(self.request.POST) + except formencode.Invalid as errors: + log.exception('Failed to add new pattern') + error = errors + h.flash(_('Invalid issue tracker pattern: {}'.format(error)), + category='error') + raise HTTPFound(h.route_path('admin_settings_issuetracker')) + if form: for uid in form.get('delete_patterns', []): settings_model.delete_entries(uid) diff --git a/rhodecode/apps/repository/tests/test_repo_issue_tracker.py b/rhodecode/apps/repository/tests/test_repo_issue_tracker.py --- a/rhodecode/apps/repository/tests/test_repo_issue_tracker.py +++ b/rhodecode/apps/repository/tests/test_repo_issue_tracker.py @@ -58,11 +58,11 @@ class TestRepoIssueTracker(object): 'edit_repo_issuetracker_update', repo_name=backend.repo.repo_name) post_data = { 'new_pattern_pattern_0': pattern, - 'new_pattern_url_0': 'url', + 'new_pattern_url_0': 'http://url', 'new_pattern_prefix_0': 'prefix', 'new_pattern_description_0': 'description', 'new_pattern_pattern_1': another_pattern, - 'new_pattern_url_1': 'url1', + 'new_pattern_url_1': '/url1', 'new_pattern_prefix_1': 'prefix1', 'new_pattern_description_1': 'description1', 'csrf_token': csrf_token @@ -84,7 +84,7 @@ class TestRepoIssueTracker(object): extra_environ=xhr_header, params=data) assert response.body == \ - 'example of prefix replacement' + 'example of prefix replacement' @request.addfinalizer def cleanup(): @@ -106,7 +106,7 @@ class TestRepoIssueTracker(object): 'edit_repo_issuetracker_update', repo_name=backend.repo.repo_name) post_data = { 'new_pattern_pattern_0': pattern, - 'new_pattern_url_0': 'url', + 'new_pattern_url_0': '/url', 'new_pattern_prefix_0': 'prefix', 'new_pattern_description_0': 'description', 'uid': old_uid, diff --git a/rhodecode/apps/repository/views/repo_settings_issue_trackers.py b/rhodecode/apps/repository/views/repo_settings_issue_trackers.py --- a/rhodecode/apps/repository/views/repo_settings_issue_trackers.py +++ b/rhodecode/apps/repository/views/repo_settings_issue_trackers.py @@ -22,6 +22,7 @@ import logging from pyramid.httpexceptions import HTTPFound from pyramid.view import view_config +import formencode from rhodecode.apps._base import RepoAppView from rhodecode.lib import audit_logger @@ -116,7 +117,17 @@ class RepoSettingsIssueTrackersView(Repo repo_settings.inherit_global_settings = inherited Session().commit() - form = IssueTrackerPatternsForm()().to_python(self.request.POST) + try: + form = IssueTrackerPatternsForm()().to_python(self.request.POST) + except formencode.Invalid as errors: + log.exception('Failed to add new pattern') + error = errors + h.flash(_('Invalid issue tracker pattern: {}'.format(error)), + category='error') + raise HTTPFound( + h.route_path('edit_repo_issuetracker', + repo_name=self.db_repo_name)) + if form: self._update_patterns(form, repo_settings) diff --git a/rhodecode/model/validators.py b/rhodecode/model/validators.py --- a/rhodecode/model/validators.py +++ b/rhodecode/model/validators.py @@ -1081,6 +1081,9 @@ def ValidAuthPlugins(): def ValidPattern(): class _Validator(formencode.validators.FancyValidator): + messages = { + 'bad_format': _(u'Url must start with http or /'), + } def _to_python(self, value, state): patterns = [] @@ -1096,7 +1099,6 @@ def ValidPattern(): values = { 'issuetracker_pat': value.get(_field('pattern')), - 'issuetracker_pat': value.get(_field('pattern')), 'issuetracker_url': value.get(_field('url')), 'issuetracker_pref': value.get(_field('prefix')), 'issuetracker_desc': value.get(_field('description')) @@ -1108,6 +1110,14 @@ def ValidPattern(): and values['issuetracker_url']) if has_required_fields: + # validate url that it starts with http or / + # otherwise it can lead to JS injections + # e.g specifig javascript: + if not values['issuetracker_url'].startswith(('http', '/')): + raise formencode.Invalid( + self.message('bad_format', state), + value, state) + settings = [ ('_'.join((key, new_uid)), values[key], 'unicode') for key in values]