Changeset - 64d41568507c
[Not reviewed]
stable
0 3 0
Mads Kiilerich (kiilerix) - 12 months ago 2018-05-29 10:25:59
mads@kiilerich.com
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/
3 files changed with 15 insertions and 10 deletions:
0 comments (0 inline, 0 general)
kallithea/model/repo.py
Show inline comments
...
 
@@ -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
kallithea/model/repo_group.py
Show inline comments
...
 
@@ -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)
 

	
kallithea/tests/api/api_base.py
Show inline comments
...
 
@@ -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)
0 comments (0 inline, 0 general)