Fix a bunch of open redirect

btcpayserver · Jan 31, 2023 · c2cfa17 · c2cfa17
1 parent 1431329
commit c2cfa17
Show file tree

Hide file tree

Showing 15 changed files with 37 additions and 24 deletions.
diff --git a/BTCPayServer.Tests/SeleniumTester.cs b/BTCPayServer.Tests/SeleniumTester.cs
@@ -203,7 +203,6 @@ public Mnemonic GenerateWallet(string cryptoCode = "BTC", string seed = "", bool
         {
             var isImport = !string.IsNullOrEmpty(seed);
             GoToWalletSettings(cryptoCode);
-
             // Replace previous wallet case
             if (Driver.PageSource.Contains("id=\"ChangeWalletLink\""))
             {

diff --git a/BTCPayServer/Extensions/UrlHelperExtensions.cs b/BTCPayServer/Extensions/UrlHelperExtensions.cs
@@ -1,4 +1,5 @@
 
+using System;
 using BTCPayServer;
 using BTCPayServer.Client.Models;
 using BTCPayServer.Controllers;
@@ -10,6 +11,18 @@ namespace Microsoft.AspNetCore.Mvc
 {
     public static class UrlHelperExtensions
     {
+#nullable enable
+        public static string? EnsureLocal(this IUrlHelper helper, string? url, HttpRequest? httpRequest = null)
+        {
+            if (url is null || helper.IsLocalUrl(url))
+                return url;
+            if (httpRequest is null)
+                return null;
+            if (Uri.TryCreate(url, UriKind.Absolute, out var r) && r.Host.Equals(httpRequest.Host.Host))
+                return url;
+            return null;
+        }
+#nullable restore
         public static string EmailConfirmationLink(this LinkGenerator urlHelper, string userId, string code, string scheme, HostString host, string pathbase)
         {
             return urlHelper.GetUriByAction(nameof(UIAccountController.ConfirmEmail), "UIAccount",

diff --git a/BTCPayServer/Models/StoreViewModels/RecoverySeedBackupViewModel.cs b/BTCPayServer/Models/StoreViewModels/RecoverySeedBackupViewModel.cs
@@ -1,3 +1,4 @@
+using System;
 using System.Collections.Generic;
 using System.ComponentModel.DataAnnotations;
 using Microsoft.AspNetCore.Http;
@@ -16,7 +17,7 @@ public class RecoverySeedBackupViewModel
 
         public string[] Words
         {
-            get => Mnemonic.Split((char[])null, System.StringSplitOptions.RemoveEmptyEntries);
+            get => Mnemonic?.Split((char[])null, System.StringSplitOptions.RemoveEmptyEntries) ?? Array.Empty<string>();
         }
     }
 }
diff --git a/BTCPayServer/Views/Shared/ConfirmModal.cshtml b/BTCPayServer/Views/Shared/ConfirmModal.cshtml
@@ -32,7 +32,7 @@
 
         @if (!string.IsNullOrEmpty(Model.Action))
         {
-            <form id="ConfirmForm" method="post" action="@actionUrl" rel="noreferrer noopener">
+            <form id="ConfirmForm" method="post" action="@Url.EnsureLocal(actionUrl)" rel="noreferrer noopener">
                 <div class="modal-body pt-0" id="ConfirmText" hidden>
                     <label for="ConfirmInput" class="form-label">Confirm the action by typing <strong id="ConfirmInputText"></strong>:</label>
                     <input id="ConfirmInput" class="form-control"/>

diff --git a/BTCPayServer/Views/Shared/PostRedirect.cshtml b/BTCPayServer/Views/Shared/PostRedirect.cshtml
@@ -38,7 +38,7 @@
 	}
 	else
 	{
-		<form method="post" id="postform" action="@Model.FormUrl" rel="noreferrer noopener">
+		<form method="post" id="postform" action="@Url.EnsureLocal(Model.FormUrl, this.Context.Request)" rel="noreferrer noopener">
 			@Html.AntiForgeryToken()
 			@foreach (var o in Model.FormParameters)
 			{

diff --git a/BTCPayServer/Views/UIHome/RecoverySeedBackup.cshtml b/BTCPayServer/Views/UIHome/RecoverySeedBackup.cshtml
@@ -73,7 +73,7 @@
         </div>
         @if (Model.RequireConfirm)
         {
-            <form id="RecoveryConfirmation" action="@Model.ReturnUrl" class="position-relative d-flex align-items-start justify-content-center" style="margin-top:4rem;padding-bottom: 80px" rel="noreferrer noopener">
+            <form id="RecoveryConfirmation" action="@Url.EnsureLocal(Model.ReturnUrl)" class="position-relative d-flex align-items-start justify-content-center" style="margin-top:4rem;padding-bottom: 80px" rel="noreferrer noopener">
                 <label class="form-check-label lead order-2" for="confirm">I have written down my recovery phrase and stored it in a secure location</label>
                 <input type="checkbox" class="me-3 order-1 form-check-input" id="confirm" style="margin-top:.35rem;flex-shrink:0">
                 <button type="submit" class="btn btn-primary btn-lg px-5 order-3" id="submit">Done</button>
@@ -82,7 +82,7 @@
         }
         else
         {
-            <a href="@Model.ReturnUrl" class="btn btn-primary btn-lg mt-3 px-5 order-3" id="proceed" rel="noreferrer noopener">Done</a>
+            <a href="@Url.EnsureLocal(Model.ReturnUrl)" class="btn btn-primary btn-lg mt-3 px-5 order-3" id="proceed" rel="noreferrer noopener">Done</a>
         }
     </div>
 </div>
diff --git a/BTCPayServer/Views/UIStores/WalletSettings.cshtml b/BTCPayServer/Views/UIStores/WalletSettings.cshtml
@@ -52,7 +52,7 @@
                            data-bs-toggle="modal"
                            data-bs-target="#ConfirmModal"
                            data-title="Replace @Model.CryptoCode wallet"
-                           data-description="@Html.Encode(ViewData["ReplaceDescription"])"
+                           data-description="@ViewData["ReplaceDescription"]"
                            data-confirm="Setup new wallet"
                            data-confirm-input="REPLACE">
                             Replace wallet
@@ -64,7 +64,7 @@
                                     data-bs-toggle="modal"
                                     data-bs-target="#ConfirmModal"
                                     data-title="Remove @Model.CryptoCode wallet"
-                                    data-description="@Html.Encode(ViewData["RemoveDescription"])"
+                                    data-description="@ViewData["RemoveDescription"]"
                                     data-confirm="Remove"
                                     data-confirm-input="REMOVE">Remove wallet</button>
                         </form>

diff --git a/BTCPayServer/Views/UIWallets/SignWithSeed.cshtml b/BTCPayServer/Views/UIWallets/SignWithSeed.cshtml
@@ -11,11 +11,11 @@
 @section Navbar {
     @if (backUrl != null)
     {
-        <a href="@backUrl" id="GoBack">
+        <a href="@Url.EnsureLocal(backUrl)" id="GoBack">
             <vc:icon symbol="back" />
         </a>
     }
-    <a href="@cancelUrl" id="CancelWizard" class="cancel">
+    <a href="@Url.EnsureLocal(cancelUrl)" id="CancelWizard" class="cancel">
         <vc:icon symbol="close" />
     </a>
 }

diff --git a/BTCPayServer/Views/UIWallets/WalletPSBT.cshtml b/BTCPayServer/Views/UIWallets/WalletPSBT.cshtml
@@ -11,11 +11,11 @@
 @section Navbar {
     @if (backUrl != null)
     {
-        <a href="@backUrl" id="GoBack">
+        <a href="@Url.EnsureLocal(backUrl)" id="GoBack">
             <vc:icon symbol="back" />
         </a>
     }
-    <a href="@cancelUrl" id="CancelWizard" class="cancel">
+    <a href="@Url.EnsureLocal(cancelUrl)" id="CancelWizard" class="cancel">
         <vc:icon symbol="close" />
     </a>
 }

diff --git a/BTCPayServer/Views/UIWallets/WalletPSBTCombine.cshtml b/BTCPayServer/Views/UIWallets/WalletPSBTCombine.cshtml
@@ -11,11 +11,11 @@
 @section Navbar {
     @if (backUrl != null)
     {
-        <a href="@backUrl" id="GoBack">
+        <a href="@Url.EnsureLocal(backUrl)" id="GoBack">
             <vc:icon symbol="back" />
         </a>
     }
-    <a href="@cancelUrl" id="CancelWizard" class="cancel">
+    <a href="@Url.EnsureLocal(cancelUrl)" id="CancelWizard" class="cancel">
         <vc:icon symbol="close" />
     </a>
 }

diff --git a/BTCPayServer/Views/UIWallets/WalletPSBTDecoded.cshtml b/BTCPayServer/Views/UIWallets/WalletPSBTDecoded.cshtml
@@ -75,11 +75,11 @@
 @section Navbar {
     @if (backUrl != null)
     {
-        <a href="@backUrl" id="GoBack">
+        <a href="@Url.EnsureLocal(backUrl)" id="GoBack">
             <vc:icon symbol="back" />
         </a>
     }
-    <a href="@cancelUrl" id="CancelWizard" class="cancel">
+    <a href="@Url.EnsureLocal(cancelUrl)" id="CancelWizard" class="cancel">
         <vc:icon symbol="close" />
     </a>
 }

diff --git a/BTCPayServer/Views/UIWallets/WalletReceive.cshtml b/BTCPayServer/Views/UIWallets/WalletReceive.cshtml
@@ -1,4 +1,4 @@
-@inject BTCPayServer.Services.BTCPayServerEnvironment env
+@inject BTCPayServer.Services.BTCPayServerEnvironment env
 @using BTCPayServer.Controllers
 @using BTCPayServer.Components.QRCode
 @model BTCPayServer.Controllers.WalletReceiveViewModel
@@ -15,7 +15,7 @@
 }
 
 @section Navbar {
-    <a href="@returnUrl" id="CancelWizard" class="cancel">
+    <a href="@Url.EnsureLocal(returnUrl)" id="CancelWizard" class="cancel">
         <vc:icon symbol="close" />
     </a>
 }

diff --git a/BTCPayServer/Views/UIWallets/WalletSend.cshtml b/BTCPayServer/Views/UIWallets/WalletSend.cshtml
@@ -14,11 +14,11 @@
 @section Navbar {
     @if (backUrl != null)
     {
-        <a href="@backUrl" id="GoBack">
+        <a href="@Url.EnsureLocal(backUrl)" id="GoBack">
             <vc:icon symbol="back" />
         </a>
     }
-    <a href="@cancelUrl" id="CancelWizard" class="cancel">
+    <a href="@Url.EnsureLocal(cancelUrl)" id="CancelWizard" class="cancel">
         <vc:icon symbol="close" />
     </a>
 }

diff --git a/BTCPayServer/Views/UIWallets/WalletSendVault.cshtml b/BTCPayServer/Views/UIWallets/WalletSendVault.cshtml
@@ -11,11 +11,11 @@
 @section Navbar {
     @if (backUrl != null)
     {
-        <a href="@backUrl" id="GoBack">
+        <a href="@Url.EnsureLocal(backUrl)" id="GoBack">
             <vc:icon symbol="back" />
         </a>
     }
-    <a href="@cancelUrl" id="CancelWizard" class="cancel">
+    <a href="@Url.EnsureLocal(cancelUrl)" id="CancelWizard" class="cancel">
         <vc:icon symbol="close" />
     </a>
 }

diff --git a/BTCPayServer/Views/UIWallets/WalletSigningOptions.cshtml b/BTCPayServer/Views/UIWallets/WalletSigningOptions.cshtml
@@ -12,11 +12,11 @@
 @section Navbar {
     @if (backUrl != null)
     {
-        <a href="@backUrl" id="GoBack">
+        <a href="@Url.EnsureLocal(backUrl)" id="GoBack">
             <vc:icon symbol="back" />
         </a>
     }
-    <a href="@cancelUrl" id="CancelWizard" class="cancel">
+    <a href="@Url.EnsureLocal(cancelUrl)" id="CancelWizard" class="cancel">
         <vc:icon symbol="close" />
     </a>
 }