From 6ab16561b5ee1a3dd4390696fc147a2ea21601b8 Mon Sep 17 00:00:00 2001
From: Lucas Bajolet <105649352+lbajolet-hashicorp@users.noreply.github.com>
Date: Wed, 20 Jul 2022 14:30:49 -0400
Subject: [PATCH] hcl2: fix crash on malformed overrides (#11881)

When an override block is either not an object, or if one of its
contents are not an object, Packer would crash trying to forcefully
cast the cty.Value to a cty.Collection.

To avoid this behaviour, we add extra checks that return hcl.Diagnostics
when they fail.
---
 .../fixtures/malformed_override.pkr.hcl       |  4 +
 .../malformed_override_innards.pkr.hcl        |  6 ++
 .../fixtures/well_formed_provisioner.pkr.hcl  |  9 ++
 hcl2template/types.build.provisioners.go      | 19 +++++
 hcl2template/types.build.provisioners_test.go | 82 +++++++++++++++++++
 5 files changed, 120 insertions(+)
 create mode 100644 hcl2template/fixtures/malformed_override.pkr.hcl
 create mode 100644 hcl2template/fixtures/malformed_override_innards.pkr.hcl
 create mode 100644 hcl2template/fixtures/well_formed_provisioner.pkr.hcl
 create mode 100644 hcl2template/types.build.provisioners_test.go

diff --git a/hcl2template/fixtures/malformed_override.pkr.hcl b/hcl2template/fixtures/malformed_override.pkr.hcl
new file mode 100644
index 000000000..753eefe08
--- /dev/null
+++ b/hcl2template/fixtures/malformed_override.pkr.hcl
@@ -0,0 +1,4 @@
+provisioner "shell-local" {
+  inline = ["echo 'hi'"]
+  override = "hello"
+}
diff --git a/hcl2template/fixtures/malformed_override_innards.pkr.hcl b/hcl2template/fixtures/malformed_override_innards.pkr.hcl
new file mode 100644
index 000000000..cd44f135c
--- /dev/null
+++ b/hcl2template/fixtures/malformed_override_innards.pkr.hcl
@@ -0,0 +1,6 @@
+provisioner "shell-local" {
+  inline = ["echo 'hi'"]
+  override = {
+    test = "hello"
+  }
+}
diff --git a/hcl2template/fixtures/well_formed_provisioner.pkr.hcl b/hcl2template/fixtures/well_formed_provisioner.pkr.hcl
new file mode 100644
index 000000000..b70bd15a8
--- /dev/null
+++ b/hcl2template/fixtures/well_formed_provisioner.pkr.hcl
@@ -0,0 +1,9 @@
+provisioner "shell-local" {
+  inline = ["echo 'hi'"]
+  override = {
+    test = {
+      "hello" = "new value"
+    }
+  }
+}
+
diff --git a/hcl2template/types.build.provisioners.go b/hcl2template/types.build.provisioners.go
index bce8a9d50..2c0659be0 100644
--- a/hcl2template/types.build.provisioners.go
+++ b/hcl2template/types.build.provisioners.go
@@ -104,9 +104,28 @@ func (p *Parser) decodeProvisioner(block *hcl.Block, ectx *hcl.EvalContext) (*Pr
 	}
 
 	if !b.Override.IsNull() {
+		if !b.Override.Type().IsObjectType() {
+			return nil, append(diags, &hcl.Diagnostic{
+				Severity: hcl.DiagError,
+				Summary:  "provisioner's override block must be an HCL object",
+				Subject:  block.DefRange.Ptr(),
+			})
+		}
+
 		override := make(map[string]interface{})
 		for buildName, overrides := range b.Override.AsValueMap() {
 			buildOverrides := make(map[string]interface{})
+
+			if !overrides.Type().IsObjectType() {
+				return nil, append(diags, &hcl.Diagnostic{
+					Severity: hcl.DiagError,
+					Summary: fmt.Sprintf(
+						"provisioner's override.'%s' block must be an HCL object",
+						buildName),
+					Subject: block.DefRange.Ptr(),
+				})
+			}
+
 			for option, value := range overrides.AsValueMap() {
 				buildOverrides[option] = hcl2shim.ConfigValueFromHCL2(value)
 			}
diff --git a/hcl2template/types.build.provisioners_test.go b/hcl2template/types.build.provisioners_test.go
new file mode 100644
index 000000000..0dae85a71
--- /dev/null
+++ b/hcl2template/types.build.provisioners_test.go
@@ -0,0 +1,82 @@
+package hcl2template
+
+import (
+	"testing"
+
+	"github.com/hashicorp/hcl/v2"
+)
+
+func TestPackerConfig_ParseProvisionerBlock(t *testing.T) {
+	tests := []struct {
+		name                 string
+		inputFile            string
+		expectError          bool
+		expectedErrorMessage string
+	}{
+		{
+			"success - provisioner is valid",
+			"fixtures/well_formed_provisioner.pkr.hcl",
+			false,
+			"",
+		},
+		{
+			"failure - provisioner override is malformed",
+			"fixtures/malformed_override.pkr.hcl",
+			true,
+			"provisioner's override block must be an HCL object",
+		},
+		{
+			"failure - provisioner override.test is malformed",
+			"fixtures/malformed_override_innards.pkr.hcl",
+			true,
+			"provisioner's override.'test' block must be an HCL object",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			cfg := PackerConfig{parser: getBasicParser()}
+			f, diags := cfg.parser.ParseHCLFile(test.inputFile)
+			if diags.HasErrors() {
+				t.Errorf("failed to parse input file %s", test.inputFile)
+				for _, d := range diags {
+					t.Errorf("%s", d)
+				}
+				return
+			}
+			provBlock := f.OutermostBlockAtPos(hcl.Pos{
+				Line:   1,
+				Column: 1,
+				Byte:   0,
+			})
+			_, diags = cfg.parser.decodeProvisioner(provBlock, nil)
+
+			if !diags.HasErrors() {
+				if !test.expectError {
+					return
+				}
+
+				t.Fatalf("unexpected success")
+			}
+
+			if !test.expectError {
+				for _, d := range diags {
+					t.Errorf("%s", d)
+				}
+			}
+
+			gotExpectedErr := false
+			for _, d := range diags {
+				if d.Summary == test.expectedErrorMessage {
+					gotExpectedErr = true
+				}
+
+				t.Logf("got error (expected): '%s'", d.Summary)
+			}
+
+			if !gotExpectedErr {
+				t.Errorf("never got expected error: '%s'", test.expectedErrorMessage)
+			}
+		})
+	}
+}