Add some validation to UriBuilder.Host setter (#121083)

Author: MihaZupan

src/libraries/System.Private.Uri/src/System/UriBuilder.cs

@@ -158,16 +158,40 @@ public string Password
             }
         }
 
+        /// <summary>
+        /// Problematic characters that could result in the Host component escaping into other components like the Path.
+        /// </summary>
+        private static readonly SearchValues<char> s_hostReservedChars = SearchValues.Create(@":/\?#@[]");
+        private static readonly SearchValues<char> s_hostReservedCharsExceptColon = SearchValues.Create(@"/\?#@[]");
+
         [AllowNull]
         public string Host
         {
             get => _host;
             set
             {
-                if (!string.IsNullOrEmpty(value) && value.Contains(':') && value[0] != '[')
+                if (!string.IsNullOrEmpty(value) && value.AsSpan().ContainsAny(s_hostReservedChars))
                 {
-                    //probable ipv6 address - Note: this is only supported for cases where the authority is inet-based.
-                    value = "[" + value + "]";
+                    if (value.Contains(':'))
+                    {
+                        if (!value.StartsWith('[') || !value.EndsWith(']'))
+                        {
+                            // probable ipv6 address - Note: this is only supported for cases where the authority is inet-based.
+                            value = $"[{value}]";
+                        }
+
+                        if (value.AsSpan(1, value.Length - 2).ContainsAny(s_hostReservedCharsExceptColon))
+                        {
+                            // Reject inputs like "[::]/path" or "::]/path".
+                            throw new ArgumentException(SR.net_uri_BadHostName, nameof(value));
+                        }
+                    }
+                    else
+                    {
+                        // Reject inputs like "contoso.com/path" or "[email protected]".
+                        // Nonsensical inputs will only be allowed by a custom parser with GenericUriParserOptions.GenericAuthority set.
+                        throw new ArgumentException(SR.net_uri_BadHostName, nameof(value));
+                    }
                 }
 
                 _host = value ?? string.Empty;
@@ -365,7 +389,7 @@ public override string ToString()
                 }
             }
 
-            var path = Path;
+            string path = Path;
             if (path.Length != 0)
             {
                 if (!path.StartsWith('/') && host.Length != 0)

src/libraries/System.Private.Uri/tests/FunctionalTests/UriBuilderTests.cs

@@ -88,7 +88,6 @@ public void Ctor_Uri_Null()
         [InlineData("http", "host", "http", "host")]
         [InlineData("HTTP", "host", "http", "host")]
         [InlineData("http", "[::1]", "http", "[::1]")]
-        [InlineData("https", "::1]", "https", "[::1]]")]
         [InlineData("http", "::1", "http", "[::1]")]
         [InlineData("http1:http2", "host", "http1", "host")]
         [InlineData("http", "", "http", "")]
@@ -107,7 +106,6 @@ public void Ctor_String_String(string? schemeName, string? hostName, string expe
         [InlineData("http", "host", 0, "http", "host")]
         [InlineData("HTTP", "host", 20, "http", "host")]
         [InlineData("http", "[::1]", 40, "http", "[::1]")]
-        [InlineData("https", "::1]", 60, "https", "[::1]]")]
         [InlineData("http", "::1", 80, "http", "[::1]")]
         [InlineData("http1:http2", "host", 100, "http1", "host")]
         [InlineData("http", "", 120, "http", "")]
@@ -126,7 +124,6 @@ public void Ctor_String_String_Int(string? scheme, string? host, int port, strin
         [InlineData("http", "host", 0, "/path", "http", "host", "/path")]
         [InlineData("HTTP", "host", 20, "/path1/path2", "http", "host", "/path1/path2")]
         [InlineData("http", "[::1]", 40, "/", "http", "[::1]", "/")]
-        [InlineData("https", "::1]", 60, "/path1/", "https", "[::1]]", "/path1/")]
         [InlineData("http", "::1", 80, null, "http", "[::1]", "/")]
         [InlineData("http1:http2", "host", 100, "path1", "http1", "host", "path1")]
         [InlineData("http", "", 120, "path1/path2", "http", "", "path1/path2")]
@@ -145,7 +142,6 @@ public void Ctor_String_String_Int_String(string? schemeName, string? hostName,
         [InlineData("http", "host", 0, "/path", "?query#fragment", "http", "host", "/path", "?query", "#fragment")]
         [InlineData("HTTP", "host", 20, "/path1/path2", "?query&query2=value#fragment", "http", "host", "/path1/path2", "?query&query2=value", "#fragment")]
         [InlineData("http", "[::1]", 40, "/", "#fragment?query", "http", "[::1]", "/", "", "#fragment?query")]
-        [InlineData("https", "::1]", 60, "/path1/", "?query", "https", "[::1]]", "/path1/", "?query", "")]
         [InlineData("http", "::1", 80, null, "#fragment", "http", "[::1]", "/", "", "#fragment")]
         [InlineData("http", "", 120, "path1/path2", "?#", "http", "", "path1/path2", "", "")]
         [InlineData("", "host", 140, "path1/path2/path3/", "?", "", "host", "path1/path2/path3/", "", "")]
@@ -368,6 +364,7 @@ public static IEnumerable<object[]> ToString_TestData()
             yield return new object[] { new UriBuilder() { Host = "host", Query = "query" }, "http://host/?query" };
             yield return new object[] { new UriBuilder() { Host = "host", Fragment = "fragment" }, "http://host/#fragment" };
             yield return new object[] { new UriBuilder() { Host = "host", Query = "query", Fragment = "fragment" }, "http://host/?query#fragment" };
+            yield return new object[] { new UriBuilder() { Host = "::%foo" }, "http://[::%foo]/" };
         }
 
         [Theory]
@@ -401,6 +398,51 @@ public void ToString_EncodingUserInfo(string username, string password, string e
             Assert.Equal(password, uriBuilder.Password);
         }
 
+        public static IEnumerable<object[]> InvalidHostStrings_TestData()
+        {
+            // Bool indicates whether the exception is expected to be thrown by the Host setter (true) or when creating the Uri (false)
+
+            // Presence of ':' is treated as a likely IPv6 address and the input is transformed into [host:80], which is invalid
+            yield return ["host:", false];
+            yield return ["host:80", false];
+
+            // Trailing space after IPv6 address
+            yield return ["[::] ", true];
+
+            // Invalid characters escape into other Uri components
+            yield return ["host/path", true];
+            yield return ["host?query", true];
+            yield return ["host#fragment", true];
+            yield return ["user@host", true];
+
+            yield return ["[::]/path", true];
+            yield return ["::]/path", true];
+            yield return ["::/path", true];
+            yield return ["::?query", true];
+            yield return ["::#fragment", true];
+            yield return ["[0::%/64]/path", true];
+
+            yield return ["[host]", true];
+            yield return ["[127.0.0.1]", true];
+        }
+
+        [Theory]
+        [MemberData(nameof(InvalidHostStrings_TestData))]
+        public void Host_Set_InvalidHostStrings_ThrowsArgumentException(string invalidHost, bool caughtByHostSetter)
+        {
+            var builder = new UriBuilder();
+
+            if (caughtByHostSetter)
+            {
+                AssertExtensions.Throws<ArgumentException>("value", () => builder.Host = invalidHost);
+            }
+            else
+            {
+                builder.Host = invalidHost;
+                Assert.Throws<UriFormatException>(() => builder.Uri);
+            }
+        }
+
         private static void VerifyUriBuilder(UriBuilder uriBuilder, string scheme, string userName, string password, string host, int port, string path, string query, string fragment)
         {
             Assert.Equal(scheme, uriBuilder.Scheme);