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: Registration.ToASCII() accepts empty TLD label #47182

Open
jusmarks opened this issue Jul 14, 2021 · 2 comments · May be fixed by golang/text#43
Open

x/net/idna: Registration.ToASCII() accepts empty TLD label #47182

jusmarks opened this issue Jul 14, 2021 · 2 comments · May be fixed by golang/text#43
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@jusmarks
Copy link

jusmarks commented Jul 14, 2021 •
edited

What did you do?

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

package main

import (
	"fmt"

	"golang.org/x/net/idna"
)

func main() {
	// Returns an error (as expected)
	fmt.Println(idna.Registration.ToASCII("gólang.órg.."))

	// Doesn't return an error
	fmt.Println(idna.Registration.ToASCII("golang.org.."))
}

What did you expect to see?

xn--glang-0ta.xn--rg-4ja.. idna: invalid label ""
golang.org.. idna: invalid label ""

What did you see instead?

xn--glang-0ta.xn--rg-4ja.. idna: invalid label ""
golang.org.. <nil>

It appears if the domain contains an empty TLD label but contains only ASCII characters, then no error is returned. And only for the TLD (empty labels elsewhere return an error as expected).
@cherrymui cherrymui added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 14, 2021
@cherrymui cherrymui added this to the Unreleased milestone Jul 14, 2021
@cherrymui
Copy link
Member

cc @neild @ianlancetaylor

elliotwutingfeng added a commit to elliotwutingfeng/text that referenced this issue Jun 15, 2023
@elliotwutingfeng
idna: *labelIter.next() to skip last label only if it is empty and ve…
f99f2cf 
…rifyDNSLength is false.

The existing implementation always skips the empty last label regardless of what verifyDNSLength is set to, which causes pure-ASCII domains ending with a single empty root label to be wrongly accepted when verifyDNSLength is true.

This behavior is described in the Unicode Technical Standard 46 at https://unicode.org/reports/tr46/\#ToASCII

Fixes golang/go#47182
@elliotwutingfeng
Copy link

From this section in the Unicode Technical Standard 46, both idna.Registration.ToASCII("gólang.órg..") and idna.Registration.ToASCII("golang.org..") should not be accepted since verifyDnsLength is set to true.

Currently idna.Registration.ToASCII("golang.org..") is being wrongly accepted, this can be fixed by setting *labelIter.next() to skip last label only if it is empty and verifyDNSLength is false.

I have drafted a possible fix here.

This was referenced Jun 17, 2023
Open
Open
elliotwutingfeng added a commit to elliotwutingfeng/text that referenced this issue Jun 22, 2023
@elliotwutingfeng
idna: *Profile.process to throw labelError if verifyDNSLength = true …
8e763eb 
…and s ends with trailing dot.

From UTS46, no trailing dots are allowed when ToASCII is called with verifyDNSLength = true. This negates the need for an additional l.verifyDNSLength flag in the previous commit.

See https://www.unicode.org/L2/L2021/21170-utc169-properties-recs.pdf under section F2

Fixes golang/go#47182
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
3 participants
@jusmarks @cherrymui @elliotwutingfeng