Mercurial > kallithea
changeset 7317:64d41568507c stable
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/
| author | Mads Kiilerich <mads@kiilerich.com> |
|---|---|
| date | Tue, 29 May 2018 12:25:59 +0200 |
| parents | 7d5e8894db6c |
| children | 4cca4cc6a0a9 |
| files | kallithea/model/repo.py kallithea/model/repo_group.py kallithea/tests/api/api_base.py |
| diffstat | 3 files changed, 15 insertions(+), 10 deletions(-) [+] |
line wrap: on
line diff
--- a/kallithea/model/repo.py Tue May 29 12:25:43 2018 +0200 +++ b/kallithea/model/repo.py Tue May 29 12:25:59 2018 +0200 @@ -33,6 +33,7 @@ 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 @@ 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 @@ # 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
--- a/kallithea/model/repo_group.py Tue May 29 12:25:43 2018 +0200 +++ b/kallithea/model/repo_group.py Tue May 29 12:25:59 2018 +0200 @@ -32,6 +32,7 @@ import shutil import datetime +import kallithea.lib.utils from kallithea.lib.utils2 import LazyProperty from kallithea.model import BaseModel @@ -145,6 +146,9 @@ 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 @@ 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)
--- a/kallithea/tests/api/api_base.py Tue May 29 12:25:43 2018 +0200 +++ b/kallithea/tests/api/api_base.py Tue May 29 12:25:59 2018 +0200 @@ -1016,14 +1016,6 @@ 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)