Fix panic when reconstructing Dockerfile from malformed image history

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])
 		}
 

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)
+	}
+}