issue-trackers: enforce a http or / patterns to avoid JS injections.

rhodecode/apps/admin/tests/test_admin_settings.py

@@ -620,11 +620,11 @@ class TestAdminSettingsIssueTracker(object):
         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(object):
         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(object):
         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,

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)

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 <a class="issue-tracker-link" href="url">prefix</a> replacement'
+               'example of <a class="issue-tracker-link" href="http://url">prefix</a> 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,

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(RepoAppView):
         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)
 

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 = []
@@ -1095,7 +1098,6 @@ def ValidPattern():
                         return '%s_%s_%s' % (prefix, name, new_item_id)
 
                     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')),
@@ -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:<malicios code>
+                        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]