Enhance `HumanReadableName` validation
What the developer wrote
Authored by Andrei
Enhance `HumanReadableName` validation
Add check that every label of user or domain is not longer than 63.
It better alings with validation in `dnssec_prover::rr::Name`.
What changed, and why it matters
This commit tightens validation for BIP 353 Human Readable Names in rust-lightning. It now enforces that each dot-separated piece (label) of the user or domain is between 1 and 63 bytes, matching DNS label limits. Previously only the total length and non-emptiness of the whole user/domain were checked, so a name could contain an oversized label that other DNS software would reject. The change is defensive hardening rather than a clear exploit fix, but it prevents possible interoperability failures or parsing mismatches with downstream DNS components.
Treat as a defensive hardening patch. Review whether any other name-parsing paths (e.g., deserialization, offer parsing) bypass HumanReadableName::new. Consider whether the 63-byte label limit should also be documented in public API docs and changelog. No urgent security response is indicated absent a disclosed vulnerability.
Security signals we found
Input validation hardening for DNS-style names
Enforcement of RFC 1035-style label length limits (1-63 bytes)
Prevention of oversized labels that could be rejected or mishandled by DNS libraries
Commit message frames change as aligning with another validator, not as a vulnerability fix
Evidence from the diff
The patch updates HumanReadableName::new in lightning/src/onion_message/dns_resolution.rs. It replaces the simple user.is_empty()/domain.is_empty() checks with per-label checks after splitting on ‘.’, rejecting empty labels and labels longer than 63 bytes. It also normalizes a trailing dot using strip_suffix. The total-length check (user + domain + REQUIRED_EXTRA_LEN <= 255) and Hostname validity checks remain. New unit tests cover 63-byte labels as valid and 64-byte labels as invalid, plus multi-label domains. The commit message explicitly says this aligns validation with dnssec_prover::rr::Name.
Changed components
lightning/src/onion_message/dns_resolution.rsHumanReadableName::newBIP 353 Human Readable Name parsingInspect captured patch +35 / −8
diff --git a/lightning/src/onion_message/dns_resolution.rs b/lightning/src/onion_message/dns_resolution.rs
index 54eb16b..6d9d4d6 100644
--- a/lightning/src/onion_message/dns_resolution.rs
+++ b/lightning/src/onion_message/dns_resolution.rs
@@ -191,8 +191,8 @@ const REQUIRED_EXTRA_LEN: usize = ".user._bitcoin-payment.".len() + 1;
/// A struct containing the two parts of a BIP 353 Human Readable Name - the user and domain parts.
///
-/// The `user` and `domain` parts, together, cannot exceed 231 bytes in length, and both must be
-/// non-empty.
+/// The `user` and `domain` parts combined cannot exceed 231 bytes in length;
+/// each DNS label within them must be non-empty and no longer than 63 bytes.
///
/// If you intend to handle non-ASCII `user` or `domain` parts, you must handle [Homograph Attacks]
/// and do punycode en-/de-coding yourself. This struct will always handle only plain ASCII `user`
@@ -211,16 +211,21 @@ pub struct HumanReadableName {
impl HumanReadableName {
/// Constructs a new [`HumanReadableName`] from the `user` and `domain` parts. See the
/// struct-level documentation for more on the requirements on each.
- pub fn new(user: &str, mut domain: &str) -> Result<HumanReadableName, ()> {
+ pub fn new(user: &str, domain: &str) -> Result<HumanReadableName, ()> {
// First normalize domain and remove the optional trailing `.`
- if domain.ends_with('.') {
- domain = &domain[..domain.len() - 1];
- }
+ let domain = domain.strip_suffix(".").unwrap_or(domain);
if user.len() + domain.len() + REQUIRED_EXTRA_LEN > 255 {
return Err(());
}
- if user.is_empty() || domain.is_empty() {
- return Err(());
+ for label in user.split('.') {
+ if label.is_empty() || label.len() > 63 {
+ return Err(());
+ }
+ }
+ for label in domain.split('.') {
+ if label.is_empty() || label.len() > 63 {
+ return Err(());
+ }
}
if !Hostname::str_is_valid_hostname(&user) || !Hostname::str_is_valid_hostname(&domain) {
return Err(());
@@ -558,6 +563,28 @@ mod tests {
);
}
+ #[test]
+ fn test_hrn_validation() {
+ assert!(HumanReadableName::new("user", "example.com").is_ok());
+ assert!(HumanReadableName::new("user", "example.com.").is_ok());
+
+ assert!(HumanReadableName::new("user!", "example.com").is_err());
+ assert!(HumanReadableName::new("user.", "example.com").is_err());
+ assert!(HumanReadableName::new("user", "exa mple.com").is_err());
+ assert!(HumanReadableName::new("", "example.com").is_err());
+ assert!(HumanReadableName::new("user", "").is_err());
+
+ let max_label = "a".repeat(63);
+ assert!(HumanReadableName::new(&max_label, "example.com").is_ok());
+
+ let long_label = "a".repeat(64);
+ assert!(HumanReadableName::new(&long_label, "example.com").is_err());
+ let domain_with_long_label = format!("{long_label}.com");
+ assert!(HumanReadableName::new("user", &domain_with_long_label).is_err());
+ let huge_domain = format!("{max_label}.{max_label}.{max_label}.{max_label}");
+ assert!(HumanReadableName::new("user", &huge_domain).is_err());
+ }
+
#[test]
#[cfg(feature = "dnssec")]
fn test_expiry() {
Why this scored 40/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.