Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uri.TryCreate (and ctor) behave weird when using the IPv6 format #97939

Open
WGroenestein opened this issue Feb 4, 2024 · 6 comments
Open

Uri.TryCreate (and ctor) behave weird when using the IPv6 format #97939

WGroenestein opened this issue Feb 4, 2024 · 6 comments
Assignees
Milestone

Comments

@WGroenestein
Copy link

Description

When using the IPv6 format for the host and it contains a % with other stuff at the end of the IPv6 block

  1. it is seen a valid (from what I understand it should not be considered a valid IPv6 format), the HostNameType is set to IPv6
  2. several properties / method results of the Uri class will not include these characters (e.g. Host and AbsoluteUri), while others do (e.g. IdnHost)

Reproduction Steps

if (Uri.TryCreate("http://[::1%zyx]:8080/", UriKind.Absolute, out Uri uri))
{
	Console.WriteLine(uri.Authority);
	Console.WriteLine(uri.Host);
	Console.WriteLine(uri.IdnHost);
	Console.WriteLine(uri.GetLeftPart(UriPartial.Authority));
	Console.WriteLine(uri.AbsoluteUri);
	Console.WriteLine(uri.HostNameType);
}
else
{
	Console.WriteLine("Invalid");
}
[::1]:8080
[::1]
::1%zyx
http://[::1]:8080
http://[::1]:8080/
IPv6

Expected behavior

I would expect the provided input to be seen as invalid (similar as is done with the "IPv4" version http://127.0.0.1%zyx:8080/).

If it is considered a valid format, I would expect the uri properties / method results to be consistent and not omit part of the user input

Actual behavior

See the description / repro steps

Regression?

I only checked the behavior on .NET 6/7/8 and they all seems to show the same behavior

Known Workarounds

No response

Configuration

8.0.100
Windows 11 23H2 22631.3007
x64
I didn't check any other configurations

Other information

I have a feeling it might be related to #45194. If this is the case however since in the URI format IPv6 is encapsulated in the [] I would expect the validation of URI to be stricter

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 4, 2024
@ghost
Copy link

ghost commented Feb 4, 2024

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

When using the IPv6 format for the host and it contains a % with other stuff at the end of the IPv6 block

  1. it is seen a valid (from what I understand it should not be considered a valid IPv6 format), the HostNameType is set to IPv6
  2. several properties / method results of the Uri class will not include these characters (e.g. Host and AbsoluteUri), while others do (e.g. IdnHost)

Reproduction Steps

if (Uri.TryCreate("http://[::1%zyx]:8080/", UriKind.Absolute, out Uri uri))
{
	Console.WriteLine(uri.Authority);
	Console.WriteLine(uri.Host);
	Console.WriteLine(uri.IdnHost);
	Console.WriteLine(uri.GetLeftPart(UriPartial.Authority));
	Console.WriteLine(uri.AbsoluteUri);
	Console.WriteLine(uri.HostNameType);
}
else
{
	Console.WriteLine("Invalid");
}
[::1]:8080
[::1]
::1%zyx
http://[::1]:8080
http://[::1]:8080/
IPv6

Expected behavior

I would expect the provided input to be seen as invalid (similar as is done with the "IPv4" version http://127.0.0.1%zyx:8080/).

If it is considered a valid format, I would expect the uri properties / method results to be consistent and not omit part of the user input

Actual behavior

See the description / repro steps

Regression?

I only checked the behavior on .NET 6/7/8 and they all seems to show the same behavior

Known Workarounds

No response

Configuration

8.0.100
Windows 11 23H2 22631.3007
x64
I didn't check any other configurations

Other information

I have a feeling it might be related to #45194. If this is the case however since in the URI format IPv6 is encapsulated in the [] I would expect the validation of URI to be stricter

Author: WGroenestein
Assignees: -
Labels:

area-System.Net, untriaged

Milestone: -

@wfurt
Copy link
Member

wfurt commented Feb 5, 2024

For IPv6 the % is marker of link local address. IPv4 does no have such concept. I think the inconsistency is there for historical reasons. While the ScopeId is needed for Socket operation it can break other places. Changing the behavior is going to be risky IMHO. Some of this is captured in #25782.

@WGroenestein
Copy link
Author

Ah, I was not aware this was a separate RFC, so that explain a lot. However it is certainly unexpected that input is whiped away for some properties and not others. From a breaking change perspective, maybe it would be worth it to extend UriCreationOptions with a or several options to reject such values so you can opt-in to having a more predictable behavior/outcome. (maybe it could even be made in such a way you can specify to only allow a domain, or IPv4 or IPv6 (with or without ScopeId) etc)

@MihaZupan
Copy link
Member

Is this causing you actual problems, or was it just an inconsistency you noticed?

While we don't like the inconsistency, changing behavior here would break people.

maybe it would be worth it to extend UriCreationOptions with a or several options to reject such values

What would the use case be here?
If you want to reject link local addresses, you could do so after parsing the Uri.
E.g. uri.HostNameType == UriHostNameType.IPv6 && uri.IdnHost.Contains('%')

@MihaZupan MihaZupan added the needs-author-action An issue or pull request that requires more info or actions from the author. label Feb 16, 2024
@ghost
Copy link

ghost commented Feb 16, 2024

This issue has been marked needs-author-action and may be missing some important information.

@WGroenestein
Copy link
Author

Is this causing you actual problems, or was it just an inconsistency you noticed?
It is not causing me any problems. as you mention I can create my own helpers to address this, it was just very surprising behavior to me, and I think it might be to others as well. And Uri is used all over the web stack so it propagates.

While we don't like the inconsistency, changing behavior here would break people.

The reason I proposed the addition(s) to the UriCreationOptions is because I was under the impression that the UriCreationOptions overload was created to make it possible to extend the options in an opt-in manner not introducing breaking changes to the behavior.

The reason I encountered it is because I was looking into the OWASP SSRF prevention Case 2 and it mentions DNS pinning (or rebinding) and though about implement such a strategy through the SocketsHttpHandler::ConnectCallback callback. In that callback an DnsEndPoint is provided and I was putting the code through some fuzzing tests which caught this scenario.

Later I found out that the DnsEndpoint is created in several steps Uri.IdnHost => HttpConnectionKey => HttpAuthority => DnsEndpoint, and the DnsEndpoint provided does not have the AddressFamily set, though HttpAuthority handles IPv6 specially and wraps it. Uri.IdnHost has this remark about unescaping however DnsEndpoint does not have such a remark, it is not very easy to figure out in my opinion.

@ghost ghost removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Feb 17, 2024
@antonfirsov antonfirsov removed the untriaged New issue has not been triaged by the area owner label Apr 9, 2024
@antonfirsov antonfirsov added this to the Future milestone Apr 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants