diff options
author | John Cai <jcai@gitlab.com> | 2020-01-29 19:30:06 +0300 |
---|---|---|
committer | John Cai <jcai@gitlab.com> | 2020-01-29 19:30:06 +0300 |
commit | 1e6359b1d26ce7ff75d65a6c8816b0d5f9dc13df (patch) | |
tree | 872e01f1ad0320d050330f882995f27b141f5251 | |
parent | 320132fa34c61ab0868aca80c02a18be86d44bb3 (diff) | |
parent | 029e4a13e340fe7d6e7b68b42671d3db7a1e7edf (diff) |
Merge branch 'jc-fix-rebase-refspec' into 'master'
Validate bad branches for UserRebase and UserRebaseConfirmable
Closes #1654
See merge request gitlab-org/gitaly!1735
-rw-r--r-- | changelogs/unreleased/jc-fix-rebase-refspec.yml | 5 | ||||
-rw-r--r-- | internal/service/operations/rebase.go | 12 | ||||
-rw-r--r-- | internal/service/operations/rebase_test.go | 17 |
3 files changed, 34 insertions, 0 deletions
diff --git a/changelogs/unreleased/jc-fix-rebase-refspec.yml b/changelogs/unreleased/jc-fix-rebase-refspec.yml new file mode 100644 index 000000000..f746c013a --- /dev/null +++ b/changelogs/unreleased/jc-fix-rebase-refspec.yml @@ -0,0 +1,5 @@ +--- +title: Validate bad branches for UserRebase and UserRebaseConfirmable +merge_request: 1735 +author: +type: security diff --git a/internal/service/operations/rebase.go b/internal/service/operations/rebase.go index f27da06f5..4346217ac 100644 --- a/internal/service/operations/rebase.go +++ b/internal/service/operations/rebase.go @@ -6,6 +6,7 @@ import ( "context" "errors" + "gitlab.com/gitlab-org/gitaly/internal/git" "gitlab.com/gitlab-org/gitaly/internal/helper" "gitlab.com/gitlab-org/gitaly/internal/rubyserver" "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb" @@ -77,6 +78,9 @@ func (s *server) userRebaseConfirmable(stream gitalypb.OperationService_UserReba ) } +// ErrInvalidBranch indicates a branch name is invalid +var ErrInvalidBranch = errors.New("invalid branch name") + func validateUserRebaseConfirmableHeader(header *gitalypb.UserRebaseConfirmableRequest_Header) error { if header.GetRepository() == nil { return errors.New("empty Repository") @@ -106,6 +110,10 @@ func validateUserRebaseConfirmableHeader(header *gitalypb.UserRebaseConfirmableR return errors.New("empty RemoteBranch") } + if err := git.ValidateRevision(header.GetRemoteBranch()); err != nil { + return ErrInvalidBranch + } + return nil } @@ -158,5 +166,9 @@ func validateUserRebaseRequest(req *gitalypb.UserRebaseRequest) error { return errors.New("empty RemoteBranch") } + if err := git.ValidateRevision(req.GetRemoteBranch()); err != nil { + return ErrInvalidBranch + } + return nil } diff --git a/internal/service/operations/rebase_test.go b/internal/service/operations/rebase_test.go index b98dca223..c064b02c1 100644 --- a/internal/service/operations/rebase_test.go +++ b/internal/service/operations/rebase_test.go @@ -146,6 +146,10 @@ func TestFailedRebaseUserRebaseConfirmableRequestDueToInvalidHeader(t *testing.T desc: "empty RemoteBranch", req: buildHeaderRequest(testRepo, rebaseUser, "1", branchName, branchSha, testRepoCopy, ""), }, + { + desc: "invalid branch name", + req: buildHeaderRequest(testRepo, rebaseUser, "1", branchName, branchSha, testRepoCopy, "+dev:master"), + }, } for _, tc := range testCases { @@ -619,6 +623,19 @@ func TestFailedUserRebaseRequestDueToValidations(t *testing.T) { }, code: codes.InvalidArgument, }, + { + desc: "invalid remote branch", + request: &gitalypb.UserRebaseRequest{ + Repository: testRepo, + User: rebaseUser, + RebaseId: "1", + Branch: []byte("some-branch"), + BranchSha: "38008cb17ce1466d8fec2dfa6f6ab8dcfe5cf49e", + RemoteRepository: testRepoCopy, + RemoteBranch: []byte("+dev:master"), + }, + code: codes.InvalidArgument, + }, } for _, testCase := range testCases { |