diff --git a/src/PSProxmoxVE/Cmdlets/Network/RemovePveSdnVnetCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Network/RemovePveSdnVnetCmdlet.cs index 08d4721..82092b8 100644 --- a/src/PSProxmoxVE/Cmdlets/Network/RemovePveSdnVnetCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Network/RemovePveSdnVnetCmdlet.cs @@ -1,5 +1,5 @@ using System.Management.Automation; -using PSProxmoxVE.Core.Client; +using PSProxmoxVE.Core.Services; namespace PSProxmoxVE.Cmdlets.Network { @@ -15,6 +15,7 @@ namespace PSProxmoxVE.Cmdlets.Network { /// The VNet identifier to remove. [Parameter(Mandatory = true, Position = 0, ValueFromPipelineByPropertyName = true, HelpMessage = "The SDN VNet name.")] + [ValidatePattern(@"\A[A-Za-z0-9][A-Za-z0-9._-]*\z")] public string Vnet { get; set; } = string.Empty; protected override void ProcessRecord() @@ -24,10 +25,10 @@ namespace PSProxmoxVE.Cmdlets.Network var session = GetSession(); RequireVersion(session, "SDN", 6, 2, 8, 0); - using var client = new PveHttpClient(session); + var service = new NetworkService(); WriteVerbose($"Removing SDN VNet '{Vnet}'..."); - client.DeleteAsync($"cluster/sdn/vnets/{Vnet}").GetAwaiter().GetResult(); + service.RemoveSdnVnet(session, Vnet); } } } diff --git a/src/PSProxmoxVE/Cmdlets/Network/RemovePveSdnZoneCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Network/RemovePveSdnZoneCmdlet.cs index 73aa216..785277c 100644 --- a/src/PSProxmoxVE/Cmdlets/Network/RemovePveSdnZoneCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Network/RemovePveSdnZoneCmdlet.cs @@ -1,5 +1,5 @@ using System.Management.Automation; -using PSProxmoxVE.Core.Client; +using PSProxmoxVE.Core.Services; namespace PSProxmoxVE.Cmdlets.Network { @@ -16,6 +16,7 @@ namespace PSProxmoxVE.Cmdlets.Network { /// The zone identifier to remove. [Parameter(Mandatory = true, Position = 0, ValueFromPipelineByPropertyName = true, HelpMessage = "The SDN zone name.")] + [ValidatePattern(@"\A[A-Za-z0-9][A-Za-z0-9._-]*\z")] public string Zone { get; set; } = string.Empty; protected override void ProcessRecord() @@ -25,10 +26,10 @@ namespace PSProxmoxVE.Cmdlets.Network var session = GetSession(); RequireVersion(session, "SDN", 6, 2, 8, 0); - using var client = new PveHttpClient(session); + var service = new NetworkService(); WriteVerbose($"Removing SDN zone '{Zone}'..."); - client.DeleteAsync($"cluster/sdn/zones/{Zone}").GetAwaiter().GetResult(); + service.RemoveSdnZone(session, Zone); } } } diff --git a/src/PSProxmoxVE/Cmdlets/Storage/RemovePveStorageCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Storage/RemovePveStorageCmdlet.cs index abb9546..6e347cf 100644 --- a/src/PSProxmoxVE/Cmdlets/Storage/RemovePveStorageCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Storage/RemovePveStorageCmdlet.cs @@ -1,5 +1,5 @@ using System.Management.Automation; -using PSProxmoxVE.Core.Client; +using PSProxmoxVE.Core.Services; namespace PSProxmoxVE.Cmdlets.Storage { @@ -18,6 +18,7 @@ namespace PSProxmoxVE.Cmdlets.Storage { /// The storage identifier to remove. [Parameter(Mandatory = true, Position = 0, ValueFromPipelineByPropertyName = true, HelpMessage = "The storage pool name.")] + [ValidatePattern(@"\A[A-Za-z0-9][A-Za-z0-9._-]*\z")] public string Storage { get; set; } = string.Empty; protected override void ProcessRecord() @@ -26,10 +27,10 @@ namespace PSProxmoxVE.Cmdlets.Storage return; var session = GetSession(); - using var client = new PveHttpClient(session); + var service = new StorageService(); WriteVerbose($"Removing storage '{Storage}'..."); - client.DeleteAsync($"storage/{Storage}").GetAwaiter().GetResult(); + service.RemoveStorage(session, Storage); } } } diff --git a/tests/PSProxmoxVE.Core.Tests/Client/PveHttpClientPathEscapingTests.cs b/tests/PSProxmoxVE.Core.Tests/Client/PveHttpClientPathEscapingTests.cs new file mode 100644 index 0000000..79b85c6 --- /dev/null +++ b/tests/PSProxmoxVE.Core.Tests/Client/PveHttpClientPathEscapingTests.cs @@ -0,0 +1,78 @@ +using System; +using System.Collections.Generic; +using System.Net; +using System.Net.Http; +using System.Reflection; +using System.Threading; +using System.Threading.Tasks; +using PSProxmoxVE.Core.Authentication; +using PSProxmoxVE.Core.Client; +using Xunit; + +namespace PSProxmoxVE.Core.Tests.Client +{ + public class PveHttpClientPathEscapingTests + { + private static void SetInnerHttpClient(PveHttpClient client, HttpClient newInner) + { + var field = typeof(PveHttpClient).GetField("_httpClient", + BindingFlags.Instance | BindingFlags.NonPublic)!; + ((HttpClient)field.GetValue(client)!).Dispose(); + field.SetValue(client, newInner); + } + + private static (PveHttpClient client, ScriptedHandler handler) NewClient( + params (HttpStatusCode status, string body)[] responses) + { + var session = new PveSession("pve.example.com", 8006, false, + "root@pam!token=aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"); + var client = new PveHttpClient(session); + var handler = new ScriptedHandler(responses); + SetInnerHttpClient(client, new HttpClient(handler)); + return (client, handler); + } + + [Fact] + public async Task DeleteAsync_WithEscapedPathSegment_DoesNotCollapseAcrossTheRealUriParser() + { + var maliciousName = "../access/users/root@pam!t"; + var (client, handler) = NewClient( + (HttpStatusCode.OK, "{\"data\":null}")); + + using (client) + { + await client.DeleteAsync($"storage/{Uri.EscapeDataString(maliciousName)}"); + } + + Assert.Single(handler.Uris); + var uri = handler.Uris[0]; + Assert.Contains("storage/..%2Faccess", uri); + Assert.DoesNotContain("storage/../", uri); + } + + private sealed class ScriptedHandler : HttpMessageHandler + { + private readonly (HttpStatusCode status, string body)[] _responses; + private int _index; + + public List Uris { get; } = new List(); + + public ScriptedHandler((HttpStatusCode status, string body)[] responses) + { + _responses = responses; + } + + protected override async Task SendAsync( + HttpRequestMessage request, CancellationToken cancellationToken) + { + Uris.Add(request.RequestUri!.ToString()); + + if (_index >= _responses.Length) + throw new InvalidOperationException("ScriptedHandler ran out of responses."); + + var (status, body) = _responses[_index++]; + return new HttpResponseMessage(status) { Content = new StringContent(body) }; + } + } + } +} diff --git a/tests/PSProxmoxVE.Core.Tests/Services/NetworkServiceTests.cs b/tests/PSProxmoxVE.Core.Tests/Services/NetworkServiceTests.cs new file mode 100644 index 0000000..ff8db6a --- /dev/null +++ b/tests/PSProxmoxVE.Core.Tests/Services/NetworkServiceTests.cs @@ -0,0 +1,62 @@ +using Moq; +using Xunit; +using PSProxmoxVE.Core.Authentication; +using PSProxmoxVE.Core.Client; +using PSProxmoxVE.Core.Services; + +namespace PSProxmoxVE.Core.Tests.Services +{ + public class NetworkServiceTests + { + private readonly Mock _mockClient; + private readonly NetworkService _service; + private readonly PveSession _session; + + public NetworkServiceTests() + { + _mockClient = new Mock(); + _service = new NetworkService(_mockClient.Object); + _session = new PveSession( + "pve.example.com", + 8006, + skipCertificateCheck: true, + apiToken: "root@pam!test=aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"); + } + + // ----------------------------------------------------------------- + // RemoveSdnZone + // ----------------------------------------------------------------- + + [Fact] + public void RemoveSdnZone_EscapesPathTraversalInName() + { + // Arrange + _mockClient.Setup(c => c.DeleteAsync("cluster/sdn/zones/..%2Faccess%2Fusers%2Fx")) + .ReturnsAsync(@"{""data"":null}"); + + // Act + _service.RemoveSdnZone(_session, "../access/users/x"); + + // Assert + _mockClient.Verify(c => c.DeleteAsync("cluster/sdn/zones/..%2Faccess%2Fusers%2Fx"), Times.Once); + } + + // ----------------------------------------------------------------- + // RemoveSdnVnet + // ----------------------------------------------------------------- + + [Fact] + public void RemoveSdnVnet_EscapesPathTraversalInName() + { + // Arrange + _mockClient.Setup(c => c.DeleteAsync("cluster/sdn/vnets/..%2Faccess%2Fusers%2Fx")) + .ReturnsAsync(@"{""data"":null}"); + + // Act + _service.RemoveSdnVnet(_session, "../access/users/x"); + + // Assert + _mockClient.Verify(c => c.DeleteAsync("cluster/sdn/vnets/..%2Faccess%2Fusers%2Fx"), Times.Once); + } + } +} diff --git a/tests/PSProxmoxVE.Core.Tests/Services/StorageServiceTests.cs b/tests/PSProxmoxVE.Core.Tests/Services/StorageServiceTests.cs index 97d5a8c..f2c78c1 100644 --- a/tests/PSProxmoxVE.Core.Tests/Services/StorageServiceTests.cs +++ b/tests/PSProxmoxVE.Core.Tests/Services/StorageServiceTests.cs @@ -246,6 +246,20 @@ namespace PSProxmoxVE.Core.Tests.Services _mockClient.Verify(c => c.DeleteAsync("storage/nfs-backup"), Times.Once); } + [Fact] + public void RemoveStorage_EscapesPathTraversalInName() + { + // Arrange + _mockClient.Setup(c => c.DeleteAsync("storage/..%2Faccess%2Fusers%2Fx")) + .ReturnsAsync(@"{""data"":null}"); + + // Act + _service.RemoveStorage(_session, "../access/users/x"); + + // Assert + _mockClient.Verify(c => c.DeleteAsync("storage/..%2Faccess%2Fusers%2Fx"), Times.Once); + } + [Fact] public void RemoveStorage_NullSession_ThrowsArgumentNullException() {