From d8eee492dfd4e544d2cd39f355730c3a96679cc2 Mon Sep 17 00:00:00 2001
From: Jonathon Jongsma <jjongsma@redhat.com>
Date: Wed, 16 Oct 2024 17:05:31 -0500
Subject: [PATCH] Don't crash when parsing vm yaml from virt-v2v

Make sure to test pointer types for nil before using them. Because yaml
unmarshalling tries to parse as much as it can and ignores extra fields in
both the struct and the data stream, we can end up with a struct that
successfully parses the input stream but is only partially initialized.
Because we're parsing this data from another source, we need to be
defensive and ensure that the data is initialized before using it.

Fixes: https://issues.redhat.com/browse/MTV-1577

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
---
 pkg/controller/plan/util/kubevirtvmparser.go  | 16 +++++---
 .../plan/util/kubevirtvmparser_test.go        |  2 +
 .../plan/util/testdata/new_format_none.yml    | 37 +++++++++++++++++++
 .../plan/util/testdata/old_format_none.yml    | 37 +++++++++++++++++++
 4 files changed, 87 insertions(+), 5 deletions(-)
 create mode 100644 pkg/controller/plan/util/testdata/new_format_none.yml
 create mode 100644 pkg/controller/plan/util/testdata/old_format_none.yml

diff --git a/pkg/controller/plan/util/kubevirtvmparser.go b/pkg/controller/plan/util/kubevirtvmparser.go
index b106ef11d..a0f14381a 100644
--- a/pkg/controller/plan/util/kubevirtvmparser.go
+++ b/pkg/controller/plan/util/kubevirtvmparser.go
@@ -66,11 +66,17 @@ func GetFirmwareFromYaml(yamlData []byte) (string, error) {
 	if err := yaml.Unmarshal(yamlData, &cnvVm); err != nil {
 		return "", err
 	}
-	if cnvVm.Spec.Template.Spec.Domain.Firmware.Bootloader.BIOS != nil {
-		return "bios", nil
-	}
-	if cnvVm.Spec.Template.Spec.Domain.Firmware.Bootloader.EFI != nil {
-		return "uefi", nil
+
+	if cnvVm.Spec.Template != nil &&
+		cnvVm.Spec.Template.Spec.Domain.Firmware != nil &&
+		cnvVm.Spec.Template.Spec.Domain.Firmware.Bootloader != nil {
+
+		if cnvVm.Spec.Template.Spec.Domain.Firmware.Bootloader.BIOS != nil {
+			return "bios", nil
+		}
+		if cnvVm.Spec.Template.Spec.Domain.Firmware.Bootloader.EFI != nil {
+			return "uefi", nil
+		}
 	}
 
 	log.Info("Firmware type was not detected")
diff --git a/pkg/controller/plan/util/kubevirtvmparser_test.go b/pkg/controller/plan/util/kubevirtvmparser_test.go
index 3819b4c3c..1178a9374 100644
--- a/pkg/controller/plan/util/kubevirtvmparser_test.go
+++ b/pkg/controller/plan/util/kubevirtvmparser_test.go
@@ -13,6 +13,8 @@ func TestKubevirtVmParser(t *testing.T) {
 	testFile(t, "new_format_efi.yml", "uefi")
 	testFile(t, "old_format_bios.yml", "bios")
 	testFile(t, "old_format_efi.yml", "uefi")
+	testFile(t, "old_format_none.yml", "")
+	testFile(t, "new_format_none.yml", "")
 }
 
 func testFile(t *testing.T, filename, expectedFormat string) {
diff --git a/pkg/controller/plan/util/testdata/new_format_none.yml b/pkg/controller/plan/util/testdata/new_format_none.yml
new file mode 100644
index 000000000..512fedd1f
--- /dev/null
+++ b/pkg/controller/plan/util/testdata/new_format_none.yml
@@ -0,0 +1,37 @@
+---
+# generated by virt-v2v 2.5.8fedora=40,release=1.fc40
+apiVersion: kubevirt.io/v1
+kind: VirtualMachine
+metadata:
+  name: mnecas-win-2022
+  labels:
+    libguestfs.org/virt-v2v-version: "2.5.8"
+    libguestfs.org/genid: f2fc75e8-4961-1425-4740-fcf2bc09685a
+    libguestfs.org/osinfo: win2k22
+    libguestfs.org/source: vmware
+spec:
+  template:
+    spec:
+      domain:
+        firmware:
+          bootloader:
+        resources:
+          clock:
+            timer:
+              hpet:
+                present: false
+              hyperv: {}
+              pit:
+                tickPolicy: delay
+              rtc:
+                tickPolicy: catchup
+            utc: {}
+          requests:
+            memory: 8192Mi
+          features:
+        cpu:
+          sockets: 2
+          cores: 2
+          thread: 1
+      terminationGracePeriodSeconds: 0
+
diff --git a/pkg/controller/plan/util/testdata/old_format_none.yml b/pkg/controller/plan/util/testdata/old_format_none.yml
new file mode 100644
index 000000000..582632b5a
--- /dev/null
+++ b/pkg/controller/plan/util/testdata/old_format_none.yml
@@ -0,0 +1,37 @@
+---
+# generated by virt-v2v 2.5.8fedora=40,release=1.fc40
+apiVersion: kubevirt.io/v1
+kind: VirtualMachine
+metadata:
+  name: mnecas-win-2022
+  labels:
+    libguestfs.org/virt-v2v-version: "2.5.8"
+    libguestfs.org/genid: 4f3c8a7c-3464-c9b0-cf79-08325a6bf1e8
+    libguestfs.org/osinfo: win2k22
+    libguestfs.org/source: kvm
+spec:
+  template:
+    spec:
+      domain:
+        os:
+        resources:
+          clock:
+            timer:
+              hpet:
+                present: false
+              hyperv: {}
+              pit:
+                tickPolicy: delay
+              rtc:
+                tickPolicy: catchup
+            utc: {}
+          requests:
+            memory: 8192Mi
+          features:
+            acpi: {}
+            apic: {}
+        cpu:
+          sockets: 2
+          cores: 2
+          thread: 1
+      terminationGracePeriodSeconds: 0