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

Jump to bottom

x/net/idna: VerifyDNSLength is not rejecting empty labels #58778

Open
zeroizerz opened this issue Feb 28, 2023 · 4 comments
Open

x/net/idna: VerifyDNSLength is not rejecting empty labels #58778

zeroizerz opened this issue Feb 28, 2023 · 4 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@zeroizerz
Copy link

What did you do?

https://go.dev/play/p/YZC2LJAeSLm

What did you expect to see?

Result consistent with https://unicode.org/reports/tr46/#ToASCII

If the VerifyDnsLength flag is true, then verify DNS length restrictions.

The length of each label is from 1 to 63.

profile.ToASCII(www..com.) = www..com. idna: invalid label "www..com."
profile.ToASCII(www..ćóḿ.) = www..xn--kda3b580m. idna: invalid label "www..ćóḿ."
profile.ToASCII(www.example..) = www.example.. idna: invalid label ""
profile.ToASCII(www.éxamplé..) = www.xn--xampl-9raf.. idna: invalid label ""

What did you see instead?

profile.ToASCII(www..com.) = www..com. idna: invalid label "www..com."
profile.ToASCII(www..ćóḿ.) = www..xn--kda3b580m. idna: invalid label "www..ćóḿ."
profile.ToASCII(www.éxamplé..) = www.xn--xampl-9raf.. idna: invalid label ""
@dmitshur
Copy link
Contributor

CC @neild, @ianlancetaylor.

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 28, 2023
@dmitshur dmitshur added this to the Unreleased milestone Feb 28, 2023
@elliotwutingfeng
Copy link

This is likely a duplicate of #47182

@zeroizerz
Copy link
Author

@elliotwutingfeng Thank you for identifying the source of the problem. This fixes the empty TLD with root label case (www.example.com..) in #47182, but actually the root label is also disallowed (www.example.com.):

When VerifyDnsLength is true, the empty root label is disallowed.

So I made a mistake in my original post, the output should be:

profile.ToASCII(www.example.com.) = www.example.com. idna: invalid label "www.example.com."
profile.ToASCII(www.éxamplé.ćóḿ.) = www.xn--xampl-9raf.xn--kda3b580m. idna: invalid label "www.éxamplé.ćóḿ."
profile.ToASCII(www..com.) = www..com. idna: invalid label "www..com."
profile.ToASCII(www..ćóḿ.) = www..xn--kda3b580m. idna: invalid label "www..ćóḿ."
profile.ToASCII(www.example..) = www.example.. idna: invalid label ""
profile.ToASCII(www.éxamplé..) = www.xn--xampl-9raf.. idna: invalid label ""

So both this issue and #47182 should be resolved by just disallowing the root label when VerifyDNSLength is true. Here is a corrected playground test: https://go.dev/play/p/FvuwfgkvzqP

@elliotwutingfeng
Copy link

Thanks for the update, I've made a PR to fix this at golang/text#43

@zeroizerz zeroizerz mentioned this issue Jun 22, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@dmitshur @elliotwutingfeng @zeroizerz