Disallow possibly malicious revisions starting with `-`

2 files changed, 27 insertions, 10 deletions

diff --git a/internal/service/commit/between.go b/internal/service/commit/between.go
index 9bbc76198..f2e0a2d3a 100644
--- a/internal/service/commit/between.go
+++ b/internal/service/commit/between.go
@@ -1,6 +1,7 @@
 package commit
 
 import (
+	"bytes"
 	"fmt"
 	"strings"
 
@@ -70,24 +71,28 @@ func gitLog(writer lines.Sender, repo *pb.Repository, from string, to string) er
 	return nil
 }
 
-func validateCommitsBetweenRequest(in *pb.CommitsBetweenRequest) error {
-	if len(in.GetFrom()) == 0 {
-		return fmt.Errorf("empty From")
+func parseCommitsBetweenRevision(revision []byte) (string, error) {
+	if len(revision) == 0 {
+		return "", fmt.Errorf("empty revision")
 	}
-
-	if len(in.GetTo()) == 0 {
-		return fmt.Errorf("empty To")
+	if bytes.HasPrefix(revision, []byte("-")) {
+		return "", fmt.Errorf("revision can't start with '-'")
 	}
 
-	return nil
+	return string(revision), nil
 }
 
 func (s *server) CommitsBetween(in *pb.CommitsBetweenRequest, stream pb.CommitService_CommitsBetweenServer) error {
-	if err := validateCommitsBetweenRequest(in); err != nil {
-		return grpc.Errorf(codes.InvalidArgument, "CommitsBetween: %v", err)
+	from, err := parseCommitsBetweenRevision(in.GetFrom())
+	if err != nil {
+		return grpc.Errorf(codes.InvalidArgument, "CommitsBetween: from: %v", err)
+	}
+	to, err := parseCommitsBetweenRevision(in.GetTo())
+	if err != nil {
+		return grpc.Errorf(codes.InvalidArgument, "CommitsBetween: to: %v", err)
 	}
 
 	writer := newCommitsBetweenWriter(stream)
 
-	return gitLog(writer, in.GetRepository(), string(in.GetFrom()), string(in.GetTo()))
+	return gitLog(writer, in.GetRepository(), from, to)
 }
diff --git a/internal/service/commit/between_test.go b/internal/service/commit/between_test.go
index 5fd99f782..b7e8addfb 100644
--- a/internal/service/commit/between_test.go
+++ b/internal/service/commit/between_test.go
@@ -191,6 +191,18 @@ func TestFailedCommitsBetweenRequest(t *testing.T) {
 			to:          nil,
 			code:        codes.InvalidArgument,
 		},
+		{
+			description: "From begins with '-'",
+			from:        append([]byte("-"), from...),
+			to:          to,
+			code:        codes.InvalidArgument,
+		},
+		{
+			description: "To begins with '-'",
+			from:        from,
+			to:          append([]byte("-"), to...),
+			code:        codes.InvalidArgument,
+		},
 	}
 
 	for _, tc := range testCases {