Skip to content

Accept internationalized domain names - #3011

Open
DL6ER wants to merge 1 commit into
developmentfrom
fix/idn-domain-validation
Open

Accept internationalized domain names#3011
DL6ER wants to merge 1 commit into
developmentfrom
fix/idn-domain-validation

Conversation

@DL6ER

@DL6ER DL6ER commented Aug 8, 2026

Copy link
Copy Markdown
Member

What does this implement/fix?

valid_domain() rejects every byte above 0x7f, so an internationalized name like äste.com is refused while its punycode form xn--ste-pla.com is accepted. We build the embedded dnsmasq with HAVE_LIBIDN2 and link libidn2, so it converts such names itself - our own validator was the only thing standing in the way.

Such a name reaches us as UTF-8, and the bytes are accepted only where they form a sequence a decoder will take. utf8_sequence_len() applies the ranges of RFC 3629, so overlong encodings, UTF-16 surrogates and anything above U+10FFFF stay refused. Simply marking 0x80-0xff valid in the lookup table would have been shorter, but a per-byte table cannot express that, and it would have let arbitrary binary through as a domain name.

This affects everything going through valid_domain(), i.e., dns.hosts, dns.domain.name, webserver.domain, the reverse servers and the list endpoints. dns.cnameRecords and dns.hostRecord validate differently and already accepted such names.

The length limits are unchanged, and the ASCII characters that were invalid before still are: ex!ample.com, ex/ample.com and anything containing a newline remain rejected.

How to test the change during review

pihole-FTL --config dns.hosts '[ "2.2.2.2 äste.com steä.com" ]'

or the same through PATCH /api/config, which answered 400 with invalid hostname ("äste.com") before and 200 now. xn--ste-pla.com and plain ASCII names keep working, and 1.1.1.1 ex!ample.com is still refused.

test/run.sh covers both directions: the new Internationalized domain names are accepted, malformed UTF-8 is not case accepts two-, three- and four-byte names and refuses an overlong encoding, a surrogate, a codepoint above U+10FFFF, a truncated sequence and a stray continuation byte. utf8_sequence_len() was additionally compared against a reference decoder over all 67 million one- to four-byte inputs, with no disagreement.

Additional information

Related issue or feature (if applicable): N/A

Pull request in docs with documentation (if applicable): N/A

Checklist:

  • The code change is tested and works locally.
  • I based my code and PRs against the repository's development branch.
  • I signed off all commits. Pi-hole enforces the DCO for all contributions
  • I signed all my commits. Pi-hole requires signatures to verify authorship
  • I have read the above and my PR is ready for review.

Copilot AI lite review requested due to automatic review settings August 8, 2026 17:18
@DL6ER
DL6ER requested a review from a team as a code owner August 8, 2026 17:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates FTL’s shared valid_domain() character validation (used by config validation, API inputs, and list parsing) to stop rejecting internationalized domain names (IDN) that contain non-ASCII bytes, allowing names like äste.com to pass validation so downstream IDN handling can proceed.

Changes:

  • Broaden valid_domain_char[] to accept non-ASCII bytes (0x80–0xFF) in domain strings.
  • Update the surrounding comment to explain why non-ASCII bytes are permitted (IDN/libidn2 context).
Suppressed comments (1)

src/tools/gravity-parseList.c:43

  • Allowing the entire 0x80–0xFF range will treat bytes that can never occur in valid UTF-8 (e.g., 0xC0–0xC1 and 0xF5–0xFF) as valid domain characters. This can let corrupted/non-UTF-8 input through validation and get stored/propagated as a “valid” domain. Consider excluding the always-invalid UTF-8 byte values while still allowing IDN U-label UTF-8 bytes.
static const unsigned char valid_domain_char[256] = {
	['a' ... 'z'] = 1, ['A' ... 'Z'] = 1, ['0' ... '9'] = 1,
	['-'] = 1, ['.'] = 1, ['_'] = 1,
	[0x80 ... 0xff] = 1,
};

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/tools/gravity-parseList.c Outdated
Comment on lines +35 to +38
// Domain must not contain any character other than [a-zA-Z0-9.-_] and the bytes
// an internationalized name is written with. We build dnsmasq with libidn2 and
// it converts those to punycode itself, so rejecting them here only refused
// names the resolver handles perfectly well.
@DL6ER DL6ER added the PRIO 2 label Aug 8, 2026
@DL6ER
DL6ER force-pushed the fix/idn-domain-validation branch from 6e07366 to f855224 Compare August 9, 2026 04:40
Comment thread src/tools/gravity-parseList.c Fixed
`valid_domain()` rejected every byte above 0x7f, so `äste.com` was refused while
its punycode form `xn--ste-pla.com` passed. We build the embedded dnsmasq with
libidn2 and it converts such names itself, so our own validator was the only
thing standing in the way.

Such a name reaches us as UTF-8, and the bytes are only accepted where they
form a sequence a decoder will take. `utf8_sequence_len()` applies the ranges
of RFC 3629, which rules out overlong encodings, UTF-16 surrogates and
everything above U+10FFFF - simply marking 0x80-0xff valid in the lookup table
would have let arbitrary binary through as a domain name. Its result was
compared against a reference decoder over all 67 million one- to four-byte
inputs, with no disagreement.

Signed-off-by: DL6ER <dl6er@dl6er.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants