From 12c016567cbca41bade3925512d0c926ccaaff95 Mon Sep 17 00:00:00 2001
From: Pranav K <prkrishn@hotmail.com>
Date: Wed, 2 Sep 2020 23:37:46 -0700
Subject: [PATCH] Reject control characters in IsLocalUrl check (#25378)

Fixes https://github.com/dotnet/aspnetcore/issues/18109
---
 src/Mvc/Mvc.Core/src/Routing/UrlHelperBase.cs | 21 ++++++++--
 .../test/Routing/UrlHelperTestBase.cs         | 39 ++++++++++++++++++-
 2 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/src/Mvc/Mvc.Core/src/Routing/UrlHelperBase.cs b/src/Mvc/Mvc.Core/src/Routing/UrlHelperBase.cs
index 2cea4a9fc7..54eedb0e45 100644
--- a/src/Mvc/Mvc.Core/src/Routing/UrlHelperBase.cs
+++ b/src/Mvc/Mvc.Core/src/Routing/UrlHelperBase.cs
@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
@@ -60,7 +60,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
                 // url doesn't start with "//" or "/\"
                 if (url[1] != '/' && url[1] != '\\')
                 {
-                    return true;
+                    return !HasControlCharacter(url.AsSpan(1));
                 }
 
                 return false;
@@ -78,15 +78,30 @@ namespace Microsoft.AspNetCore.Mvc.Routing
                 // url doesn't start with "~//" or "~/\"
                 if (url[2] != '/' && url[2] != '\\')
                 {
-                    return true;
+                    return !HasControlCharacter(url.AsSpan(2));
                 }
 
                 return false;
             }
 
             return false;
+
+            static bool HasControlCharacter(ReadOnlySpan<char> readOnlySpan)
+            {
+                // URLs may not contain ASCII control characters.
+                for (var i = 0; i < readOnlySpan.Length; i++)
+                {
+                    if (char.IsControl(readOnlySpan[i]))
+                    {
+                        return true;
+                    }
+                }
+
+                return false;
+            }
         }
 
+
         /// <inheritdoc />
         public virtual string Content(string contentPath)
         {
diff --git a/src/Mvc/Mvc.Core/test/Routing/UrlHelperTestBase.cs b/src/Mvc/Mvc.Core/test/Routing/UrlHelperTestBase.cs
index a1b3da3088..a0693946bc 100644
--- a/src/Mvc/Mvc.Core/test/Routing/UrlHelperTestBase.cs
+++ b/src/Mvc/Mvc.Core/test/Routing/UrlHelperTestBase.cs
@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
@@ -288,6 +288,43 @@ namespace Microsoft.AspNetCore.Mvc.Routing
             Assert.False(result);
         }
 
+        [Theory]
+        [InlineData("\n")]
+        [InlineData("\\n")]
+        [InlineData("/\n")]
+        [InlineData("~\n")]
+        [InlineData("~/\n")]
+        public void IsLocalUrl_RejectsUrlWithNewLineAtStart(string url)
+        {
+            // Arrange
+            var helper = CreateUrlHelper(appRoot: string.Empty, host: "www.mysite.com", protocol: null);
+
+            // Act
+            var result = helper.IsLocalUrl(url);
+
+            // Assert
+            Assert.False(result);
+        }
+
+        [Theory]
+        [InlineData("/\r\nsomepath")]
+        [InlineData("~/\r\nsomepath")]
+        [InlineData("/some\npath")]
+        [InlineData("~/some\npath")]
+        [InlineData("\\path\b")]
+        [InlineData("~\\path\b")]
+        public void IsLocalUrl_RejectsUrlWithControlCharacters(string url)
+        {
+            // Arrange
+            var helper = CreateUrlHelper(appRoot: string.Empty, host: "www.mysite.com", protocol: null);
+
+            // Act
+            var result = helper.IsLocalUrl(url);
+
+            // Assert
+            Assert.False(result);
+        }
+
         [Fact]
         public void RouteUrlWithDictionary()
         {