# HG changeset patch
# User Mads Kiilerich <mads@kiilerich.com>
# Date 2018-05-29 12:25:59
# Node ID 64d41568507ce86f9fc601933f1cb0b4c8caddca
# Parent  7d5e8894db6c0b251321171de1e55965d3ba8503

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.compat import json
@@ -342,7 +343,10 @@ class RepoModel(BaseModel):
                 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'):
@@ -393,6 +397,8 @@ class RepoModel(BaseModel):
             # 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 import BaseModel
@@ -145,6 +146,9 @@ class RepoGroupModel(BaseModel):
     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)
+
             user = self._get_user(owner)
             parent_group = self._get_repo_group(parent)
             new_repo_group = RepoGroup()
@@ -296,7 +300,10 @@ class RepoGroupModel(BaseModel):
             repo_group.enable_locking = form_data['enable_locking']
 
             repo_group.parent_group = RepoGroup.get(form_data['group_parent_id'])
-            repo_group.group_name = repo_group.get_new_name(form_data['group_name'])
+            group_name = form_data['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
             self.sa.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
@@ -1016,14 +1016,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)