diff --git a/src/PSProxmoxVE.Core/Models/Vms/OvfMetadata.cs b/src/PSProxmoxVE.Core/Models/Vms/OvfMetadata.cs index eca30cb..4df6967 100644 --- a/src/PSProxmoxVE.Core/Models/Vms/OvfMetadata.cs +++ b/src/PSProxmoxVE.Core/Models/Vms/OvfMetadata.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; using System.IO; using System.Text; +using System.Text.RegularExpressions; using System.Xml; namespace PSProxmoxVE.Core.Models.Vms @@ -61,6 +62,11 @@ namespace PSProxmoxVE.Core.Models.Vms private const string RasdNs = "http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_ResourceAllocationSettingData"; private const string VssdNs = "http://schemas.dmtf.org/wbem/wscim/1/cim-schema/2/CIM_VirtualSystemSettingData"; + // ovf:href is used as a path segment in a PVE property string (import-from=storage:import/ova/href); + // PVE property strings are comma-separated, so ',' and any path separator must be rejected. + // \A/\z (not ^/$) so a trailing newline cannot sneak past the anchor under .NET's default regex options. + private static readonly Regex ValidHrefPattern = new Regex(@"\A[A-Za-z0-9._-]+\z", RegexOptions.Compiled); + /// /// Parses an OVA file (TAR archive) and extracts OVF metadata. /// @@ -106,10 +112,19 @@ namespace PSProxmoxVE.Core.Models.Vms /// /// Parses OVF XML and extracts VM metadata. /// - private static OvfMetadata ParseOvfXml(string xml) + internal static OvfMetadata ParseOvfXml(string xml) { var doc = new XmlDocument(); - doc.LoadXml(xml); + var readerSettings = new XmlReaderSettings + { + DtdProcessing = DtdProcessing.Prohibit, + XmlResolver = null + }; + using (var stringReader = new StringReader(xml)) + using (var xmlReader = XmlReader.Create(stringReader, readerSettings)) + { + doc.Load(xmlReader); + } var nsm = new XmlNamespaceManager(doc.NameTable); nsm.AddNamespace("ovf", OvfNs); @@ -158,6 +173,8 @@ namespace PSProxmoxVE.Core.Models.Vms var href = fileNode.Attributes?["ovf:href"]?.Value; if (id != null && href != null) { + if (!ValidHrefPattern.IsMatch(href) || href == "." || href == "..") + throw new InvalidDataException($"OVF descriptor references a disallowed file name: '{href}'."); fileRefs[id] = href; } } @@ -209,9 +226,9 @@ namespace PSProxmoxVE.Core.Models.Vms } break; - case 6: // Parallel SCSI HBA (sometimes used as SATA controller) case 5: // IDE Controller - case 20: // SCSI/SAS controller (storage) + case 6: // Parallel SCSI HBA + case 20: // Other storage device (VMware's SATA AHCI controller) // Controllers themselves don't produce disk entries; skip. break; @@ -296,8 +313,8 @@ namespace PSProxmoxVE.Core.Models.Vms switch (rt) { case 5: return "ide"; - case 6: return "sata"; - case 20: return "scsi"; + case 6: return "scsi"; + case 20: return "sata"; } } break; diff --git a/src/PSProxmoxVE/Cmdlets/Vms/ImportPveOvaCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Vms/ImportPveOvaCmdlet.cs index 1766a9c..32a124e 100644 --- a/src/PSProxmoxVE/Cmdlets/Vms/ImportPveOvaCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Vms/ImportPveOvaCmdlet.cs @@ -226,12 +226,38 @@ namespace PSProxmoxVE.Cmdlets.Vms if (!string.IsNullOrEmpty(metadata.OsTypeHint) && metadata.OsTypeHint != "other") vmConfig["ostype"] = metadata.OsTypeHint; - // Add disk import-from parameters (use scsi bus like PVE UI) + // Add disk import-from parameters, placed on the bus the OVF names. + // Slot counts per PVE's qemu-server schema; a bus that runs out overflows to scsi. string? firstDisk = null; + var busIndexes = new Dictionary + { + ["scsi"] = 0, + ["sata"] = 0, + ["ide"] = 0 + }; + var busMax = new Dictionary + { + ["scsi"] = 30, + ["sata"] = 5, + ["ide"] = 3 + }; for (int i = 0; i < metadata.Disks.Count; i++) { var disk = metadata.Disks[i]; - var diskSlot = $"scsi{i}"; + var bus = !string.IsNullOrEmpty(disk.BusType) && busIndexes.ContainsKey(disk.BusType) ? disk.BusType : "scsi"; + if (busIndexes[bus] > busMax[bus]) + bus = "scsi"; + if (busIndexes[bus] > busMax[bus]) + { + ThrowTerminatingError(new ErrorRecord( + new InvalidOperationException($"OVF descriptor has more disks than PVE's scsi bus can hold ({busMax["scsi"] + 1})."), + "TooManyDisks", + ErrorCategory.LimitsExceeded, + metadata.Disks)); + return; + } + var diskSlot = $"{bus}{busIndexes[bus]}"; + busIndexes[bus]++; var importFrom = $"{Storage}:import/{fileName}/{disk.FileName}"; vmConfig[diskSlot] = $"{TargetStorage}:0,import-from={importFrom}"; firstDisk ??= diskSlot; diff --git a/tests/PSProxmoxVE.Core.Tests/Models/OvfMetadataTests.cs b/tests/PSProxmoxVE.Core.Tests/Models/OvfMetadataTests.cs new file mode 100644 index 0000000..1211c30 --- /dev/null +++ b/tests/PSProxmoxVE.Core.Tests/Models/OvfMetadataTests.cs @@ -0,0 +1,104 @@ +using System.IO; +using System.Xml; +using Xunit; +using PSProxmoxVE.Core.Models.Vms; + +namespace PSProxmoxVE.Core.Tests.Models +{ + public class OvfMetadataTests + { + private const string OvfHeader = + ""; + + private static string BuildDescriptor(string href, int controllerResourceType) + { + return + OvfHeader + + "" + + "" + + "" + + "" + + "" + + "1" + + "" + controllerResourceType + "" + + "" + + "" + + "2" + + "17" + + "1" + + "ovf:/disk/vmdisk1" + + "" + + "" + + "" + + ""; + } + + [Fact] + public void ParseOvfXml_VMwareScsiController_ResourceType6_MapsToScsi() + { + var xml = BuildDescriptor("disk1.vmdk", 6); + var metadata = OvfMetadata.ParseOvfXml(xml); + + Assert.Single(metadata.Disks); + Assert.Equal("scsi", metadata.Disks[0].BusType); + } + + [Fact] + public void ParseOvfXml_VMwareSataController_ResourceType20_MapsToSata() + { + var xml = BuildDescriptor("disk1.vmdk", 20); + var metadata = OvfMetadata.ParseOvfXml(xml); + + Assert.Single(metadata.Disks); + Assert.Equal("sata", metadata.Disks[0].BusType); + } + + [Fact] + public void ParseOvfXml_HrefWithEmbeddedProperty_Throws() + { + var xml = BuildDescriptor("d.vmdk,cache=unsafe", 6); + + Assert.Throws(() => OvfMetadata.ParseOvfXml(xml)); + } + + [Fact] + public void ParseOvfXml_HrefWithPathTraversal_Throws() + { + var xml = BuildDescriptor("../x.vmdk", 6); + + Assert.Throws(() => OvfMetadata.ParseOvfXml(xml)); + } + + [Fact] + public void ParseOvfXml_HrefIsDotDot_Throws() + { + var xml = BuildDescriptor("..", 6); + + Assert.Throws(() => OvfMetadata.ParseOvfXml(xml)); + } + + [Fact] + public void ParseOvfXml_HrefIsDot_Throws() + { + var xml = BuildDescriptor(".", 6); + + Assert.Throws(() => OvfMetadata.ParseOvfXml(xml)); + } + + [Fact] + public void ParseOvfXml_DescriptorWithInternalEntity_Throws() + { + var xml = + "" + + "]>" + + OvfHeader + + "" + + ""; + + Assert.ThrowsAny(() => OvfMetadata.ParseOvfXml(xml)); + } + } +}