# HG changeset patch
# User Mads Kiilerich <mads@kiilerich.com>
# Date 2018-05-29 12:25:59
# Node ID 57a733313e4f5cc48bf4b0d7dab96e652d7dcf49
# Parent  ebc239a474a312ddfbf02534ff75695e6304199e

repos: introduce low level slug check of repo and group names

The high level web forms already slug-ify repo and repo group names. It might
thus not create the exact repo that was created, but the name will be "safe".

For API, we would rather have it fail than not doing exactly what was requested.

Thus, always verify at low level that the provided name wouldn't be modified by
slugification. This makes sure the API provide allow the same actual names as
the web UI.

This will only influence creation and renaming of repositories and repo groups.
Existing repositories will continue working as before.

This is a slight API change, but it makes the system more stable and can
prevent some security issues - especially XSS attacks.

This issue was found and reported by
Kacper Szurek
https://security.szurek.pl/

diff --git a/kallithea/model/repo.py b/kallithea/model/repo.py
--- a/kallithea/model/repo.py
+++ b/kallithea/model/repo.py
@@ -33,6 +33,7 @@ import traceback
 from datetime import datetime
 from sqlalchemy.orm import subqueryload
 
+import kallithea.lib.utils
 from kallithea.lib.utils import make_ui, is_valid_repo_uri
 from kallithea.lib.vcs.backends import get_backend
 from kallithea.lib.utils2 import LazyProperty, safe_str, safe_unicode, \
@@ -314,7 +315,10 @@ class RepoModel(object):
                 cur_repo.clone_uri = clone_uri
 
             if 'repo_name' in kwargs:
-                cur_repo.repo_name = cur_repo.get_new_name(kwargs['repo_name'])
+                repo_name = kwargs['repo_name']
+                if kallithea.lib.utils.repo_name_slug(repo_name) != repo_name:
+                    raise Exception('invalid repo name %s' % repo_name)
+                cur_repo.repo_name = cur_repo.get_new_name(repo_name)
 
             # if private flag is set, reset default permission to NONE
             if kwargs.get('repo_private'):
@@ -363,6 +367,8 @@ class RepoModel(object):
             # with name and path of group
             repo_name_full = repo_name
             repo_name = repo_name.split(self.URL_SEPARATOR)[-1]
+            if kallithea.lib.utils.repo_name_slug(repo_name) != repo_name:
+                raise Exception('invalid repo name %s' % repo_name)
 
             new_repo = Repository()
             new_repo.repo_state = state
diff --git a/kallithea/model/repo_group.py b/kallithea/model/repo_group.py
--- a/kallithea/model/repo_group.py
+++ b/kallithea/model/repo_group.py
@@ -32,6 +32,7 @@ import traceback
 import shutil
 import datetime
 
+import kallithea.lib.utils
 from kallithea.lib.utils2 import LazyProperty
 
 from kallithea.model.db import RepoGroup, Session, Ui, UserRepoGroupToPerm, \
@@ -135,6 +136,9 @@ class RepoGroupModel(object):
     def create(self, group_name, group_description, owner, parent=None,
                just_db=False, copy_permissions=False):
         try:
+            if kallithea.lib.utils.repo_name_slug(group_name) != group_name:
+                raise Exception('invalid repo group name %s' % group_name)
+
             owner = User.guess_instance(owner)
             parent_group = RepoGroup.guess_instance(parent)
             new_repo_group = RepoGroup()
@@ -290,7 +294,10 @@ class RepoGroupModel(object):
                 assert kwargs['parent_group_id'] != u'-1', kwargs # RepoGroupForm should have converted to None
                 repo_group.parent_group = RepoGroup.get(kwargs['parent_group_id'])
             if 'group_name' in kwargs:
-                repo_group.group_name = repo_group.get_new_name(kwargs['group_name'])
+                group_name = kwargs['group_name']
+                if kallithea.lib.utils.repo_name_slug(group_name) != group_name:
+                    raise Exception('invalid repo group name %s' % group_name)
+                repo_group.group_name = repo_group.get_new_name(group_name)
             new_path = repo_group.full_path
             Session().add(repo_group)
 
diff --git a/kallithea/tests/api/api_base.py b/kallithea/tests/api/api_base.py
--- a/kallithea/tests/api/api_base.py
+++ b/kallithea/tests/api/api_base.py
@@ -1063,14 +1063,6 @@ class _BaseTestApi(object):
         if repo_name == '/':
             expected = "repo group `` not found"
             self._compare_error(id_, expected, given=response.body)
-        elif repo_name in [':', '<test>']:
-            # FIXME: special characters and XSS injection should not be allowed
-            expected = {
-                'msg': 'Created new repository `%s`' % repo_name,
-                'success': True,
-                'task': None,
-            }
-            self._compare_ok(id_, expected, given=response.body)
         else:
             expected = "failed to create repository `%s`" % repo_name
             self._compare_error(id_, expected, given=response.body)
@@ -1134,7 +1126,7 @@ class _BaseTestApi(object):
 
         top_group = RepoGroup.get_by_group_name(TEST_REPO_GROUP)
         assert top_group
-        rg = fixture.create_repo_group(repo_group_basename, group_parent_id=top_group)
+        rg = fixture.create_repo_group(repo_group_basename, parent_group_id=top_group)
         Session().commit()
         RepoGroupModel().grant_user_permission(repo_group_name,
                                                self.TEST_USER_LOGIN,