From 67a1f2dda9bcb5795033b0c0ce775d931627fe17 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Fri, 5 Oct 2018 21:48:43 -0700 Subject: [PATCH 1/3] Add security text about Host header --- .../IUrlHelper.cs | 30 +++++- .../UrlHelperExtensions.cs | 96 +++++++++++++++++-- 2 files changed, 114 insertions(+), 12 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/IUrlHelper.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/IUrlHelper.cs index 33a23fef25..8a0a75db9e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/IUrlHelper.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/IUrlHelper.cs @@ -1,6 +1,7 @@ // 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 Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Routing; namespace Microsoft.AspNetCore.Mvc @@ -19,10 +20,18 @@ namespace Microsoft.AspNetCore.Mvc /// Generates a URL with an absolute path for an action method, which contains the action /// name, controller name, route values, protocol to use, host name, and fragment specified by /// . Generates an absolute URL if and - /// are non-null. + /// are non-null. See the remarks section for important security information. /// /// The context object for the generated URLs for an action method. /// The generated URL. + /// + /// + /// The value of should be a trusted value. Relying on the value of the current request + /// can allow untrusted input to influence the resulting URI unless the Host header has been validated. + /// See the deployment documentation for instructions on how to properly validate the Host header in + /// your deployment environment. + /// + /// string Action(UrlActionContext actionContext); /// @@ -65,19 +74,36 @@ namespace Microsoft.AspNetCore.Mvc /// Generates a URL with an absolute path, which contains the route name, route values, protocol to use, host /// name, and fragment specified by . Generates an absolute URL if /// and are non-null. + /// See the remarks section for important security information. /// /// The context object for the generated URLs for a route. /// The generated URL. + /// + /// + /// The value of should be a trusted value. Relying on the value of the current request + /// can allow untrusted input to influence the resulting URI unless the Host header has been validated. + /// See the deployment documentation for instructions on how to properly validate the Host header in + /// your deployment environment. + /// + /// string RouteUrl(UrlRouteContext routeContext); /// /// Generates an absolute URL for the specified and route /// , which contains the protocol (such as "http" or "https") and host name from the - /// current request. + /// current request. See the remarks section for important security information. /// /// The name of the route that is used to generate URL. /// An object that contains route values. /// The generated absolute URL. + /// + /// + /// This method uses the value of to populate the host section of the generated URI. + /// Relying on the value of the current request can allow untrusted input to influence the resulting URI unless + /// the Host header has been validated. See the deployment documentation for instructions on how to properly + /// validate the Host header in your deployment environment. + /// + /// string Link(string routeName, object values); } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/UrlHelperExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/UrlHelperExtensions.cs index e790b879ef..40c673da8c 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/UrlHelperExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/UrlHelperExtensions.cs @@ -2,9 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Diagnostics; -using Microsoft.AspNetCore.Mvc.Core; -using Microsoft.AspNetCore.Mvc.Internal; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Routing; using Microsoft.AspNetCore.Routing; @@ -108,7 +106,7 @@ namespace Microsoft.AspNetCore.Mvc /// /// Generates a URL with an absolute path for an action method, which contains the specified /// name, name, route , and - /// to use. + /// to use. See the remarks section for important security information. /// /// The . /// The name of the action method. @@ -116,6 +114,14 @@ namespace Microsoft.AspNetCore.Mvc /// An object that contains route values. /// The protocol for the URL, such as "http" or "https". /// The generated URL. + /// + /// + /// This method uses the value of to populate the host section of the generated URI. + /// Relying on the value of the current request can allow untrusted input to influence the resulting URI unless + /// the Host header has been validated. See the deployment documentation for instructions on how to properly + /// validate the Host header in your deployment environment. + /// + /// public static string Action( this IUrlHelper helper, string action, @@ -136,7 +142,7 @@ namespace Microsoft.AspNetCore.Mvc /// name, name, route , /// to use, and name. /// Generates an absolute URL if the and are - /// non-null. + /// non-null. See the remarks section for important security information. /// /// The . /// The name of the action method. @@ -145,6 +151,14 @@ namespace Microsoft.AspNetCore.Mvc /// The protocol for the URL, such as "http" or "https". /// The host name for the URL. /// The generated URL. + /// + /// + /// The value of should be a trusted value. Relying on the value of the current request + /// can allow untrusted input to influence the resulting URI unless the Host header has been validated. + /// See the deployment documentation for instructions on how to properly validate the Host header in + /// your deployment environment. + /// + /// public static string Action( this IUrlHelper helper, string action, @@ -166,7 +180,7 @@ namespace Microsoft.AspNetCore.Mvc /// name, name, route , /// to use, name, and . /// Generates an absolute URL if the and are - /// non-null. + /// non-null. See the remarks section for important security information. /// /// The . /// The name of the action method. @@ -176,6 +190,14 @@ namespace Microsoft.AspNetCore.Mvc /// The host name for the URL. /// The fragment for the URL. /// The generated URL. + /// + /// + /// The value of should be a trusted value. Relying on the value of the current request + /// can allow untrusted input to influence the resulting URI unless the Host header has been validated. + /// See the deployment documentation for instructions on how to properly validate the Host header in + /// your deployment environment. + /// + /// public static string Action( this IUrlHelper helper, string action, @@ -253,13 +275,22 @@ namespace Microsoft.AspNetCore.Mvc /// /// Generates a URL with an absolute path for the specified route and route - /// , which contains the specified to use. + /// , which contains the specified to use. See the + /// remarks section for important security information. /// /// The . /// The name of the route that is used to generate URL. /// An object that contains route values. /// The protocol for the URL, such as "http" or "https". /// The generated URL. + /// + /// + /// This method uses the value of to populate the host section of the generated URI. + /// Relying on the value of the current request can allow untrusted input to influence the resulting URI unless + /// the Host header has been validated. See the deployment documentation for instructions on how to properly + /// validate the Host header in your deployment environment. + /// + /// public static string RouteUrl( this IUrlHelper helper, string routeName, @@ -279,6 +310,7 @@ namespace Microsoft.AspNetCore.Mvc /// , which contains the specified to use and /// name. Generates an absolute URL if /// and are non-null. + /// See the remarks section for important security information. /// /// The . /// The name of the route that is used to generate URL. @@ -286,6 +318,14 @@ namespace Microsoft.AspNetCore.Mvc /// The protocol for the URL, such as "http" or "https". /// The host name for the URL. /// The generated URL. + /// + /// + /// The value of should be a trusted value. Relying on the value of the current request + /// can allow untrusted input to influence the resulting URI unless the Host header has been validated. + /// See the deployment documentation for instructions on how to properly validate the Host header in + /// your deployment environment. + /// + /// public static string RouteUrl( this IUrlHelper helper, string routeName, @@ -306,6 +346,7 @@ namespace Microsoft.AspNetCore.Mvc /// , which contains the specified to use, /// name and . Generates an absolute URL if /// and are non-null. + /// See the remarks section for important security information. /// /// The . /// The name of the route that is used to generate URL. @@ -314,6 +355,14 @@ namespace Microsoft.AspNetCore.Mvc /// The host name for the URL. /// The fragment for the URL. /// The generated URL. + /// + /// + /// The value of should be a trusted value. Relying on the value of the current request + /// can allow untrusted input to influence the resulting URI unless the Host header has been validated. + /// See the deployment documentation for instructions on how to properly validate the Host header in + /// your deployment environment. + /// + /// public static string RouteUrl( this IUrlHelper helper, string routeName, @@ -382,7 +431,8 @@ namespace Microsoft.AspNetCore.Mvc => Page(urlHelper, pageName, pageHandler, values, protocol: null); /// - /// Generates a URL with an absolute path for the specified . + /// Generates a URL with an absolute path for the specified . See the remarks section + /// for important security information. /// /// The . /// The page name to generate the url for. @@ -390,6 +440,14 @@ namespace Microsoft.AspNetCore.Mvc /// An object that contains route values. /// The protocol for the URL, such as "http" or "https". /// The generated URL. + /// + /// + /// This method uses the value of to populate the host section of the generated URI. + /// Relying on the value of the current request can allow untrusted input to influence the resulting URI unless + /// the Host header has been validated. See the deployment documentation for instructions on how to properly + /// validate the Host header in your deployment environment. + /// + /// public static string Page( this IUrlHelper urlHelper, string pageName, @@ -399,7 +457,8 @@ namespace Microsoft.AspNetCore.Mvc => Page(urlHelper, pageName, pageHandler, values, protocol, host: null, fragment: null); /// - /// Generates a URL with an absolute path for the specified . + /// Generates a URL with an absolute path for the specified . See the remarks section for + /// important security information. /// /// The . /// The page name to generate the url for. @@ -408,6 +467,14 @@ namespace Microsoft.AspNetCore.Mvc /// The protocol for the URL, such as "http" or "https". /// The host name for the URL. /// The generated URL. + /// + /// + /// The value of should be a trusted value. Relying on the value of the current request + /// can allow untrusted input to influence the resulting URI unless the Host header has been validated. + /// See the deployment documentation for instructions on how to properly validate the Host header in + /// your deployment environment. + /// + /// public static string Page( this IUrlHelper urlHelper, string pageName, @@ -418,7 +485,8 @@ namespace Microsoft.AspNetCore.Mvc => Page(urlHelper, pageName, pageHandler, values, protocol, host, fragment: null); /// - /// Generates a URL with an absolute path for the specified . + /// Generates a URL with an absolute path for the specified . See the remarks section for + /// important security information. /// /// The . /// The page name to generate the url for. @@ -428,6 +496,14 @@ namespace Microsoft.AspNetCore.Mvc /// The host name for the URL. /// The fragment for the URL. /// The generated URL. + /// + /// + /// The value of should be a trusted value. Relying on the value of the current request + /// can allow untrusted input to influence the resulting URI unless the Host header has been validated. + /// See the deployment documentation for instructions on how to properly validate the Host header in + /// your deployment environment. + /// + /// public static string Page( this IUrlHelper urlHelper, string pageName, From 956441aa68ce7516bce69fda616a0fe060c83882 Mon Sep 17 00:00:00 2001 From: "David J. Quiroga" Date: Mon, 8 Oct 2018 14:41:04 -0300 Subject: [PATCH 2/3] Ignore created URI if Assembly.CodeBase contains a fragment (#8556) * Fixes #8367 --- .../RelatedAssemblyAttribute.cs | 3 +- .../RelatedAssemblyPartTest.cs | 50 ++++++++++++++++++- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationParts/RelatedAssemblyAttribute.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationParts/RelatedAssemblyAttribute.cs index b9f6cdb2d7..294d7b9ac8 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationParts/RelatedAssemblyAttribute.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationParts/RelatedAssemblyAttribute.cs @@ -115,7 +115,8 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationParts internal static string GetAssemblyLocation(Assembly assembly) { - if (Uri.TryCreate(assembly.CodeBase, UriKind.Absolute, out var result) && result.IsFile) + if (Uri.TryCreate(assembly.CodeBase, UriKind.Absolute, out var result) && + result.IsFile && string.IsNullOrWhiteSpace(result.Fragment)) { return result.LocalPath; } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationParts/RelatedAssemblyPartTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationParts/RelatedAssemblyPartTest.cs index efce8dfaad..6819818bf1 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationParts/RelatedAssemblyPartTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationParts/RelatedAssemblyPartTest.cs @@ -80,7 +80,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationParts { // Arrange var destination = Path.Combine(AssemblyDirectory, "RelatedAssembly.dll"); - var codeBase = "file://x/file/Assembly.dll"; + var codeBase = "file://x:/file/Assembly.dll"; var expected = new Uri(codeBase).LocalPath; var assembly = new TestAssembly { @@ -109,6 +109,54 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationParts Assert.Equal(expected, actual); } + [Fact] + public void GetAssemblyLocation_CodeBase_HasPoundCharacterUnixPath() + { + var destination = Path.Combine(AssemblyDirectory, "RelatedAssembly.dll"); + var expected = @"/etc/#NIN/dotnetcore/tryx/try1.dll"; + var assembly = new TestAssembly + { + CodeBaseSettable = "file:///etc/#NIN/dotnetcore/tryx/try1.dll", + LocationSettable = expected, + }; + + // Act + var actual = RelatedAssemblyAttribute.GetAssemblyLocation(assembly); + Assert.Equal(expected, actual); + } + + [Fact] + public void GetAssemblyLocation_CodeBase_HasPoundCharacterUNCPath() + { + var destination = Path.Combine(AssemblyDirectory, "RelatedAssembly.dll"); + var expected = @"\\server\#NIN\dotnetcore\tryx\try1.dll"; + var assembly = new TestAssembly + { + CodeBaseSettable = "file://server/#NIN/dotnetcore/tryx/try1.dll", + LocationSettable = expected, + }; + + // Act + var actual = RelatedAssemblyAttribute.GetAssemblyLocation(assembly); + Assert.Equal(expected, actual); + } + + [Fact] + public void GetAssemblyLocation_CodeBase_HasPoundCharacterDOSPath() + { + var destination = Path.Combine(AssemblyDirectory, "RelatedAssembly.dll"); + var expected = @"C:\#NIN\dotnetcore\tryx\try1.dll"; + var assembly = new TestAssembly + { + CodeBaseSettable = "file:///C:/#NIN/dotnetcore/tryx/try1.dll", + LocationSettable = expected, + }; + + // Act + var actual = RelatedAssemblyAttribute.GetAssemblyLocation(assembly); + Assert.Equal(expected, actual); + } + private class TestAssembly : Assembly { public override AssemblyName GetName() From 6cee2431f1d5247ea2f5de7af5198f3c328015cc Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Sun, 7 Oct 2018 10:22:38 -0700 Subject: [PATCH 3/3] React to routing changes --- build/dependencies.props | 4 ++-- .../Routing/EndpointRoutingUrlHelperTest.cs | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/build/dependencies.props b/build/dependencies.props index aca99b4e4b..4e7f5dcd1f 100644 --- a/build/dependencies.props +++ b/build/dependencies.props @@ -47,8 +47,8 @@ 2.2.0-preview3-35359 2.2.0-preview3-35359 2.2.0-preview3-35359 - 2.2.0-a-preview3-matcher-policy-17051 - 2.2.0-a-preview3-matcher-policy-17051 + 2.2.0-a-preview3-address-scheme-17059 + 2.2.0-a-preview3-address-scheme-17059 2.2.0-preview3-35359 2.2.0-preview3-35359 2.2.0-preview3-35359 diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/EndpointRoutingUrlHelperTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/EndpointRoutingUrlHelperTest.cs index bf423d775e..facb4e07e1 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/EndpointRoutingUrlHelperTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Routing/EndpointRoutingUrlHelperTest.cs @@ -325,7 +325,5 @@ namespace Microsoft.AspNetCore.Mvc.Routing EndpointMetadataCollection.Empty, null); } - - private class SuppressLinkGenerationMetadata : ISuppressLinkGenerationMetadata { } } }