From c5a6a54be6dc434360c27e06e236c24ca88276f4 Mon Sep 17 00:00:00 2001
From: 0xVijay <53905346+0xVijay@users.noreply.github.com>
Date: Sat, 4 Jul 2026 00:08:08 +0530
Subject: [PATCH] Prevent panic when reconstructing Dockerfile from malformed
 image history

The HEALTHCHECK/ENTRYPOINT/CMD reverse-engineering code indexes the
results of strings.SplitN without checking their length, so a malformed
value in an image's layer history makes it panic. There is no recover()
around it, so a single bad instruction aborts the whole xray/build/profile
run.

Bounds-check the slice accesses in deserialiseHealtheckInstruction and
fixJSONArray, and return an error for malformed HEALTHCHECK values so the
caller skips the instruction instead of crashing.

Signed-off-by: 0xVijay <53905346+0xVijay@users.noreply.github.com>
---
 pkg/docker/dockerfile/reverse/reverse.go      | 10 ++++--
 pkg/docker/dockerfile/reverse/reverse_test.go | 35 +++++++++++++++++++
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/pkg/docker/dockerfile/reverse/reverse.go b/pkg/docker/dockerfile/reverse/reverse.go
index 538697c55..4ff3c8b3a 100644
--- a/pkg/docker/dockerfile/reverse/reverse.go
+++ b/pkg/docker/dockerfile/reverse/reverse.go
@@ -693,7 +693,7 @@ func SaveDockerfileData(fatImageDockerfileLocation string, fatImageDockerfileLin
 
 func fixJSONArray(in string) string {
 	data := in
-	if data[0] == '[' {
+	if len(data) >= 2 && data[0] == '[' {
 		data = data[1 : len(data)-1]
 	}
 	outArray, err := shlex.Split(data)
@@ -796,6 +796,9 @@ func deserialiseHealtheckInstruction(data string) (string, *crt.HealthConfig, er
 
 		//Splits the string into two parts - first part pointer to array of string and rest of the string with } in end.
 		instParts := strings.SplitN(cleanInst, "]", 2)
+		if len(instParts) < 2 {
+			return strTest, &config, fmt.Errorf("malformed HEALTHCHECK instruction: %q", data)
+		}
 		// Cleans HEALTHCHECK part and splits the first part further
 		parts := strings.SplitN(instParts[0], " ", 2)
 		// joins the first part of the string
@@ -812,6 +815,9 @@ func deserialiseHealtheckInstruction(data string) (string, *crt.HealthConfig, er
 		instPart2 = strings.TrimSpace(instPart2)
 
 		paramParts := strings.SplitN(instPart2, " ", 4)
+		if len(paramParts) < 4 {
+			return strTest, &config, fmt.Errorf("malformed HEALTHCHECK parameters: %q", data)
+		}
 		for i, param := range paramParts {
 			paramParts[i] = strings.Trim(param, "\"'")
 		}
@@ -884,7 +890,7 @@ func deserialiseHealtheckInstruction(data string) (string, *crt.HealthConfig, er
 					err = fmt.Errorf("got an invalid escape sequence: %s", rawRetries)
 				}
 			}
-		} else {
+		} else if len(rawRetries) > 0 {
 			retries = int64((rawRetries)[0])
 		}
 
diff --git a/pkg/docker/dockerfile/reverse/reverse_test.go b/pkg/docker/dockerfile/reverse/reverse_test.go
index 8726346d0..bd664fc7a 100644
--- a/pkg/docker/dockerfile/reverse/reverse_test.go
+++ b/pkg/docker/dockerfile/reverse/reverse_test.go
@@ -108,3 +108,38 @@ func TestHealthCheck(t *testing.T) {
 		assert.Equal(t, testData.reconstructedHealthcheck, res)
 	}
 }
+
+func TestHealthCheckMalformed(t *testing.T) {
+	// A malformed HEALTHCHECK value from image history must not panic.
+	// It should return an error and let the caller skip the instruction.
+	malformed := []string{
+		`HEALTHCHECK &{[`,
+		`HEALTHCHECK &{[CMD]}`,
+		`HEALTHCHECK &{[CMD] 5s 10s 0s`,
+		`HEALTHCHECK &{["CMD"]`,
+	}
+
+	for _, in := range malformed {
+		require.NotPanics(t, func() {
+			_, _, err := deserialiseHealtheckInstruction(in)
+			assert.Error(t, err, "expected an error for %q", in)
+		}, "input %q must not panic", in)
+	}
+}
+
+func TestDockerfileFromHistoryDataMalformed(t *testing.T) {
+	// Crafted image history instructions must not crash the reconstruction.
+	crafted := []string{
+		`/bin/sh -c #(nop)  HEALTHCHECK &{[`,
+		`/bin/sh -c #(nop)  HEALTHCHECK &{[CMD] 5s 10s 0s`,
+		`/bin/sh -c #(nop)  ENTRYPOINT [`,
+		`/bin/sh -c #(nop)  CMD [`,
+	}
+
+	for _, createdBy := range crafted {
+		data := fmt.Sprintf(`[{"CreatedBy":%q,"Size":0}]`, createdBy)
+		require.NotPanics(t, func() {
+			_, _ = DockerfileFromHistoryData(data)
+		}, "CreatedBy %q must not panic", createdBy)
+	}
+}