From 7b254b62bd83923f701805ea878adfb7e0151aef Mon Sep 17 00:00:00 2001 From: CarloToso <105941898+CarloToso@users.noreply.github.com> Date: Fri, 7 Apr 2023 20:54:40 +0200 Subject: [PATCH 1/7] Fix Bug --- .../utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs index c078faaed5c..99358c3b3f9 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs @@ -592,8 +592,12 @@ protected override void ProcessRecord() OutFile = null; } + // Skip insecure redirection detection if the URIs are relative + bool destinationIsHttps = response.RequestMessage.RequestUri.IsAbsoluteUri && response.RequestMessage.RequestUri.Scheme == "https"; + bool originIsHttp = response.Headers.Location?.IsAbsoluteUri && response.Headers.Location?.Scheme == "http"; + // Detect insecure redirection - if (!AllowInsecureRedirect && response.RequestMessage.RequestUri.Scheme == "https" && response.Headers.Location?.Scheme == "http") + if (!AllowInsecureRedirect && destinationIsHttps && originIsHttp) { ErrorRecord er = new(new InvalidOperationException(), "InsecureRedirection", ErrorCategory.InvalidOperation, request); er.ErrorDetails = new ErrorDetails(WebCmdletStrings.InsecureRedirection); From 696ed7e50af1a73b95bf902b4437fd18f1976346 Mon Sep 17 00:00:00 2001 From: CarloToso <105941898+CarloToso@users.noreply.github.com> Date: Fri, 7 Apr 2023 20:59:36 +0200 Subject: [PATCH 2/7] fix variable names --- .../utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs index 99358c3b3f9..e0fe591180c 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs @@ -593,11 +593,11 @@ protected override void ProcessRecord() } // Skip insecure redirection detection if the URIs are relative - bool destinationIsHttps = response.RequestMessage.RequestUri.IsAbsoluteUri && response.RequestMessage.RequestUri.Scheme == "https"; - bool originIsHttp = response.Headers.Location?.IsAbsoluteUri && response.Headers.Location?.Scheme == "http"; + bool originIsHttps = response.RequestMessage.RequestUri.IsAbsoluteUri && response.RequestMessage.RequestUri.Scheme == "https"; + bool destinationIsHttp = response.Headers.Location?.IsAbsoluteUri && response.Headers.Location?.Scheme == "http"; // Detect insecure redirection - if (!AllowInsecureRedirect && destinationIsHttps && originIsHttp) + if (!AllowInsecureRedirect && originIsHttps && destinationIsHttp) { ErrorRecord er = new(new InvalidOperationException(), "InsecureRedirection", ErrorCategory.InvalidOperation, request); er.ErrorDetails = new ErrorDetails(WebCmdletStrings.InsecureRedirection); From 71a11ea148c1165345f97075d91785d09284c039 Mon Sep 17 00:00:00 2001 From: CarloToso <105941898+CarloToso@users.noreply.github.com> Date: Fri, 7 Apr 2023 22:28:27 +0200 Subject: [PATCH 3/7] fix build --- .../utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs index e0fe591180c..adea0828899 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs @@ -594,7 +594,7 @@ protected override void ProcessRecord() // Skip insecure redirection detection if the URIs are relative bool originIsHttps = response.RequestMessage.RequestUri.IsAbsoluteUri && response.RequestMessage.RequestUri.Scheme == "https"; - bool destinationIsHttp = response.Headers.Location?.IsAbsoluteUri && response.Headers.Location?.Scheme == "http"; + bool destinationIsHttp = response.Headers.Location is not null && response.Headers.Location.IsAbsoluteUri && response.Headers.Location.Scheme == "http"; // Detect insecure redirection if (!AllowInsecureRedirect && originIsHttps && destinationIsHttp) From 84c7aa6111b056ab5c974e1cf5799f072a9cc101 Mon Sep 17 00:00:00 2001 From: CarloToso <105941898+CarloToso@users.noreply.github.com> Date: Sat, 8 Apr 2023 09:31:23 +0200 Subject: [PATCH 4/7] follow suggestion --- .../Common/WebRequestPSCmdlet.Common.cs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs index adea0828899..10694f4b9ec 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs @@ -592,16 +592,19 @@ protected override void ProcessRecord() OutFile = null; } - // Skip insecure redirection detection if the URIs are relative - bool originIsHttps = response.RequestMessage.RequestUri.IsAbsoluteUri && response.RequestMessage.RequestUri.Scheme == "https"; - bool destinationIsHttp = response.Headers.Location is not null && response.Headers.Location.IsAbsoluteUri && response.Headers.Location.Scheme == "http"; - // Detect insecure redirection - if (!AllowInsecureRedirect && originIsHttps && destinationIsHttp) + if (!AllowInsecureRedirect) { - ErrorRecord er = new(new InvalidOperationException(), "InsecureRedirection", ErrorCategory.InvalidOperation, request); - er.ErrorDetails = new ErrorDetails(WebCmdletStrings.InsecureRedirection); - ThrowTerminatingError(er); + // Skip insecure redirection detection if the URIs are relative + bool originIsHttps = response.RequestMessage.RequestUri.IsAbsoluteUri && response.RequestMessage.RequestUri.Scheme == "https"; + bool destinationIsHttp = response.Headers.Location is not null && response.Headers.Location.IsAbsoluteUri && response.Headers.Location.Scheme == "http"; + + if (originIsHttps && destinationIsHttp) + { + ErrorRecord er = new(new InvalidOperationException(), "InsecureRedirection", ErrorCategory.InvalidOperation, request); + er.ErrorDetails = new ErrorDetails(WebCmdletStrings.InsecureRedirection); + ThrowTerminatingError(er); + } } if (ShouldCheckHttpStatus && !_isSuccess) From 5449cef2a679fecc6da0fdce76448c15424f2e64 Mon Sep 17 00:00:00 2001 From: CarloToso <105941898+CarloToso@users.noreply.github.com> Date: Sat, 8 Apr 2023 22:00:40 +0200 Subject: [PATCH 5/7] Add tests --- .../WebCmdlets.Tests.ps1 | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 index 5f1e6e4bae3..b07afbe3627 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/WebCmdlets.Tests.ps1 @@ -1149,6 +1149,15 @@ Describe "Invoke-WebRequest tests" -Tags "Feature", "RequireAdminOnWindows" { $result = ExecuteWebCommand -command $command $result.Error.FullyQualifiedErrorId | Should -Be "InsecureRedirection,Microsoft.PowerShell.Commands.InvokeWebRequestCommand" } + + It "Validate Invoke-WebRequest Https to Http (No Scheme) redirect without -AllowInsecureRedirect" { + $httpUri = Get-WebListenerUrl -Test 'Get' + $uri = Get-WebListenerUrl -Test 'Redirect' -Https -Query @{destination = $httpUri.Authority} + $command = "Invoke-WebRequest -Uri '$uri' -SkipCertificateCheck" + + $result = ExecuteWebCommand -command $command + $result.Error.FullyQualifiedErrorId | Should -Be "WebCmdletWebResponseException,Microsoft.PowerShell.Commands.InvokeWebRequestCommand" + } } @@ -3100,6 +3109,15 @@ Describe "Invoke-RestMethod tests" -Tags "Feature", "RequireAdminOnWindows" { $result.Error.FullyQualifiedErrorId | Should -Be "InsecureRedirection,Microsoft.PowerShell.Commands.InvokeRestMethodCommand" } + It "Validate Invoke-RestMethod Https to Http (No Scheme) redirect without -AllowInsecureRedirect" { + $httpUri = Get-WebListenerUrl -Test 'Get' + $uri = Get-WebListenerUrl -Test 'Redirect' -Https -Query @{destination = $httpUri.Authority} + $command = "Invoke-RestMethod -Uri '$uri' -SkipCertificateCheck" + + $result = ExecuteWebCommand -command $command + $result.Error.FullyQualifiedErrorId | Should -Be "WebCmdletWebResponseException,Microsoft.PowerShell.Commands.InvokeRestMethodCommand" + } + #endregion Redirect tests Context "Invoke-RestMethod SkipHeaderVerification Tests" { From e78a20f5a9c23f0494eb79ea942f5f255c99b30d Mon Sep 17 00:00:00 2001 From: CarloToso <105941898+CarloToso@users.noreply.github.com> Date: Wed, 12 Apr 2023 01:08:12 +0200 Subject: [PATCH 6/7] Add comments @daxian-dbw --- .../utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs index 10694f4b9ec..0c43660ea65 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs @@ -592,10 +592,11 @@ protected override void ProcessRecord() OutFile = null; } - // Detect insecure redirection + // Detect insecure redirection. if (!AllowInsecureRedirect) { - // Skip insecure redirection detection if the URIs are relative + // We are skipping detection if the URIs are relative, because Scheme property is not supported on relative URIs. + // If we skip the check, there may be a failure due to https-to-http redirect being forbidden by default. bool originIsHttps = response.RequestMessage.RequestUri.IsAbsoluteUri && response.RequestMessage.RequestUri.Scheme == "https"; bool destinationIsHttp = response.Headers.Location is not null && response.Headers.Location.IsAbsoluteUri && response.Headers.Location.Scheme == "http"; From 0a6608fc3d6c2508da16588df11d08400506f6d0 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Tue, 11 Apr 2023 17:20:34 -0700 Subject: [PATCH 7/7] Update WebRequestPSCmdlet.Common.cs --- .../utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs index 0c43660ea65..79ec2903665 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebRequestPSCmdlet.Common.cs @@ -595,8 +595,8 @@ protected override void ProcessRecord() // Detect insecure redirection. if (!AllowInsecureRedirect) { - // We are skipping detection if the URIs are relative, because Scheme property is not supported on relative URIs. - // If we skip the check, there may be a failure due to https-to-http redirect being forbidden by default. + // We will skip detection if either of the URIs is relative, because the 'Scheme' property is not supported on a relative URI. + // If we have to skip the check, an error may be thrown later if it's actually an insecure https-to-http redirect. bool originIsHttps = response.RequestMessage.RequestUri.IsAbsoluteUri && response.RequestMessage.RequestUri.Scheme == "https"; bool destinationIsHttp = response.Headers.Location is not null && response.Headers.Location.IsAbsoluteUri && response.Headers.Location.Scheme == "http";