From ba9869c0301fc2d4aae5e61af563be0ba578c3e3 Mon Sep 17 00:00:00 2001 From: Veiko Aasa Date: Mon, 28 Oct 2019 18:44:26 +0300 Subject: [PATCH] gitweb: Validate repository name also in actions script This prevents writing to an arbitrary directory if running actions script as a root user. - Included tests for invalid names Signed-off-by: Veiko Aasa [sunil@medhas.org Parametrize the test case] Signed-off-by: Sunil Mohan Adapa Reviewed-by: Sunil Mohan Adapa --- actions/gitweb | 9 +++++++++ plinth/modules/gitweb/forms.py | 22 +++++++++++++++++----- plinth/modules/gitweb/tests/test_gitweb.py | 13 +++++++++++++ 3 files changed, 39 insertions(+), 5 deletions(-) diff --git a/actions/gitweb b/actions/gitweb index 3771781f8..05b96c8c8 100755 --- a/actions/gitweb +++ b/actions/gitweb @@ -27,6 +27,7 @@ import shutil import subprocess from plinth import action_utils +from plinth.modules.gitweb.forms import validate_repository from plinth.modules.gitweb.manifest import GIT_REPO_PATH @@ -168,6 +169,8 @@ def _set_access_status(repo, status): def subcommand_rename_repo(arguments): """Rename a repository.""" + validate_repository(arguments.oldname) + validate_repository(arguments.newname) oldpath = os.path.join(GIT_REPO_PATH, arguments.oldname + '.git') newpath = os.path.join(GIT_REPO_PATH, arguments.newname + '.git') os.rename(oldpath, newpath) @@ -175,21 +178,25 @@ def subcommand_rename_repo(arguments): def subcommand_set_repo_description(arguments): """Set description of the repository.""" + validate_repository(arguments.name) _set_repo_description(arguments.name, arguments.description) def subcommand_set_repo_owner(arguments): """Set repository's owner name.""" + validate_repository(arguments.name) _set_repo_owner(arguments.name, arguments.owner) def subcommand_set_repo_access(arguments): """Set repository's access status.""" + validate_repository(arguments.name) _set_access_status(arguments.name, arguments.access) def subcommand_repo_info(arguments): """Get information about repository.""" + validate_repository(arguments.name) repo_path = os.path.join(GIT_REPO_PATH, arguments.name + '.git') if not os.path.exists(repo_path): raise RuntimeError('Repository not found') @@ -203,6 +210,7 @@ def subcommand_repo_info(arguments): def subcommand_create_repo(arguments): """Create a new git repository.""" + validate_repository(arguments.name) os.chdir(GIT_REPO_PATH) repo_name = arguments.name + '.git' subprocess.check_call(['git', 'init', '--bare', repo_name]) @@ -216,6 +224,7 @@ def subcommand_create_repo(arguments): def subcommand_delete_repo(arguments): """Delete a git repository.""" + validate_repository(arguments.name) repo_path = os.path.join(GIT_REPO_PATH, arguments.name + '.git') shutil.rmtree(repo_path) diff --git a/plinth/modules/gitweb/forms.py b/plinth/modules/gitweb/forms.py index 670570a70..ff22b1e85 100644 --- a/plinth/modules/gitweb/forms.py +++ b/plinth/modules/gitweb/forms.py @@ -18,6 +18,8 @@ Django form for configuring Gitweb. """ +import re + from django import forms from django.core.exceptions import ValidationError from django.utils.translation import ugettext_lazy as _ @@ -25,13 +27,26 @@ from django.utils.translation import ugettext_lazy as _ from plinth.modules import gitweb +def validate_repository(name): + """Validate a Git repository name.""" + + if not re.match(r'^[a-zA-Z0-9-._]+$', name): + raise ValidationError(_('Invalid repository name.')) + + if name.startswith(('-', '.')): + raise ValidationError(_('Invalid repository name.')) + + if name.endswith('.git.git'): + raise ValidationError(_('Invalid repository name.')) + + class EditRepoForm(forms.Form): """Form to create and edit a new repository.""" - name = forms.RegexField( + name = forms.CharField( label=_('Name of the repository'), strip=True, - regex=r'^[a-zA-Z0-9-._]+$', + validators=[validate_repository], help_text=_( 'An alpha-numeric string that uniquely identifies a repository.'), ) @@ -62,9 +77,6 @@ class EditRepoForm(forms.Form): if name.endswith('.git'): name = name[:-4] - if (not name) or name.startswith(('-', '.')): - raise ValidationError(_('Invalid repository name.')) - for repo in gitweb.app.get_repo_list(): if name == repo['name']: raise ValidationError( diff --git a/plinth/modules/gitweb/tests/test_gitweb.py b/plinth/modules/gitweb/tests/test_gitweb.py index 2b389b5e7..cec4ee7a3 100644 --- a/plinth/modules/gitweb/tests/test_gitweb.py +++ b/plinth/modules/gitweb/tests/test_gitweb.py @@ -24,6 +24,7 @@ import pathlib from unittest.mock import patch import pytest +from django.forms import ValidationError def _action_file(): @@ -91,3 +92,15 @@ def test_actions(call_action): call_action(['delete-repo', '--name', repo_renamed]) with pytest.raises(RuntimeError, match='Repository not found'): call_action(['repo-info', '--name', repo_renamed]) + + +@pytest.mark.parametrize( + 'name', + ['.Test-repo', 'Test-repo.git.git', '/root/Test-repo', 'Test-repö']) +def test_action_create_repo_with_invalid_names(call_action, name): + """Test that creating repository with invalid names fails.""" + with pytest.raises(ValidationError): + call_action([ + 'create-repo', '--name', name, '--description', '', '--owner', '', + '--keep-ownership' + ])