Validate bad branches for UserRebase and UserRebaseConfirmable

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 {