fix: escape dots and special characters in DNS label text per RFC 1035 §5.1
Closes #36 read_qname was pushing raw label bytes directly into the output string, producing ambiguous text for labels containing dots, backslashes, or non-printable bytes. fanf2 spotted this on HN: wire format `[8]exa.mple[3]com[0]` (two labels, first containing a literal 0x2E) was rendered as `exa.mple.com`, indistinguishable from three labels. Fix both sides of the text representation per RFC 1035 §5.1: read_qname — when rendering wire bytes to text: - literal `.` within a label → `\.` - literal `\` → `\\` - bytes outside 0x21..=0x7E → `\DDD` (3-digit decimal) - printable ASCII passes through unchanged write_qname — when parsing text back to wire: - `\.` produces a literal 0x2E inside the current label (not a separator) - `\\` produces a literal 0x5C - `\DDD` produces the byte with that decimal value (0..=255) - unescaped `.` still separates labels, empty labels still skipped - rejects trailing `\`, short `\DD`, and `\DDD` > 255 Impact in practice is low — real-world domains don't contain dots in labels — but it's a correctness bug in the wire format parser that could cause round-trip failures with adversarial input. The parser is the core of the project, so correctness bugs take priority over practical impact. Adds 16 unit tests in a new `#[cfg(test)] mod tests` block covering: plain domain read/write, literal-dot escaping on both sides, backslash escaping, non-printable + space decimal escapes, full round-trip preservation, and the three rejection cases for malformed escapes. Credit: fanf2 (https://news.ycombinator.com/item?id=47612321) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Razvan Dimescu
208
src/buffer.rs
208
src/buffer.rs
@@ -84,6 +84,11 @@ impl BytePacketBuffer {
|
||||
|
||||
/// Read a qname, handling label compression (pointer jumps).
|
||||
/// Converts wire format like [3]www[6]google[3]com[0] into "www.google.com".
|
||||
///
|
||||
/// Label bytes are escaped per RFC 1035 §5.1:
|
||||
/// - literal `.` within a label → `\.`
|
||||
/// - literal `\` → `\\`
|
||||
/// - bytes outside `0x21..=0x7E` (excluding `.` and `\`) → `\DDD` (3-digit decimal)
|
||||
pub fn read_qname(&mut self, outstr: &mut String) -> Result<()> {
|
||||
let mut pos = self.pos();
|
||||
let mut jumped = false;
|
||||
@@ -121,7 +126,13 @@ impl BytePacketBuffer {
|
||||
|
||||
let str_buffer = self.get_range(pos, len as usize)?;
|
||||
for &b in str_buffer {
|
||||
outstr.push(b.to_ascii_lowercase() as char);
|
||||
let c = b.to_ascii_lowercase();
|
||||
match c {
|
||||
b'.' => outstr.push_str("\\."),
|
||||
b'\\' => outstr.push_str("\\\\"),
|
||||
0x21..=0x7E => outstr.push(c as char),
|
||||
_ => outstr.push_str(&format!("\\{:03}", c)),
|
||||
}
|
||||
}
|
||||
|
||||
delim = ".";
|
||||
@@ -163,23 +174,23 @@ impl BytePacketBuffer {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Write a qname in wire format, parsing RFC 1035 §5.1 text escapes.
|
||||
///
|
||||
/// Dots are label separators unless escaped as `\.`. `\\` yields a literal
|
||||
/// backslash, and `\DDD` (three decimal digits) yields an arbitrary byte.
|
||||
pub fn write_qname(&mut self, qname: &str) -> Result<()> {
|
||||
if qname.is_empty() || qname == "." {
|
||||
self.write_u8(0)?;
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
for label in qname.split('.') {
|
||||
let len = label.len();
|
||||
if len == 0 {
|
||||
continue; // skip empty labels from trailing dot
|
||||
}
|
||||
if len > 0x3f {
|
||||
let labels = parse_escaped_labels(qname)?;
|
||||
for label in &labels {
|
||||
if label.len() > 0x3f {
|
||||
return Err("Single label exceeds 63 characters of length".into());
|
||||
}
|
||||
|
||||
self.write_u8(len as u8)?;
|
||||
for b in label.as_bytes() {
|
||||
self.write_u8(label.len() as u8)?;
|
||||
for b in label {
|
||||
self.write_u8(*b)?;
|
||||
}
|
||||
}
|
||||
@@ -212,3 +223,180 @@ impl BytePacketBuffer {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_escaped_labels(qname: &str) -> Result<Vec<Vec<u8>>> {
|
||||
let mut labels: Vec<Vec<u8>> = Vec::new();
|
||||
let mut current: Vec<u8> = Vec::new();
|
||||
let mut chars = qname.chars();
|
||||
|
||||
while let Some(c) = chars.next() {
|
||||
if c == '\\' {
|
||||
match chars.next() {
|
||||
Some(d1) if d1.is_ascii_digit() => {
|
||||
let d2 = chars
|
||||
.next()
|
||||
.and_then(|c| c.to_digit(10))
|
||||
.ok_or("invalid \\DDD escape: expected 3 digits")?;
|
||||
let d3 = chars
|
||||
.next()
|
||||
.and_then(|c| c.to_digit(10))
|
||||
.ok_or("invalid \\DDD escape: expected 3 digits")?;
|
||||
let val = d1.to_digit(10).unwrap() * 100 + d2 * 10 + d3;
|
||||
if val > 255 {
|
||||
return Err(format!("\\DDD escape out of range: {}", val).into());
|
||||
}
|
||||
current.push(val as u8);
|
||||
}
|
||||
Some(other) => {
|
||||
let mut buf = [0u8; 4];
|
||||
current.extend_from_slice(other.encode_utf8(&mut buf).as_bytes());
|
||||
}
|
||||
None => return Err("trailing backslash in qname".into()),
|
||||
}
|
||||
} else if c == '.' {
|
||||
if !current.is_empty() {
|
||||
labels.push(std::mem::take(&mut current));
|
||||
}
|
||||
} else {
|
||||
let mut buf = [0u8; 4];
|
||||
current.extend_from_slice(c.encode_utf8(&mut buf).as_bytes());
|
||||
}
|
||||
}
|
||||
if !current.is_empty() {
|
||||
labels.push(current);
|
||||
}
|
||||
Ok(labels)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
fn roundtrip(wire: &[u8]) -> String {
|
||||
let mut buf = BytePacketBuffer::from_bytes(wire);
|
||||
let mut out = String::new();
|
||||
buf.read_qname(&mut out).unwrap();
|
||||
out
|
||||
}
|
||||
|
||||
fn write_then_read(text: &str) -> String {
|
||||
let mut buf = BytePacketBuffer::new();
|
||||
buf.write_qname(text).unwrap();
|
||||
let wire_end = buf.pos();
|
||||
buf.seek(0).unwrap();
|
||||
let mut out = String::new();
|
||||
buf.read_qname(&mut out).unwrap();
|
||||
assert_eq!(
|
||||
buf.pos(),
|
||||
wire_end,
|
||||
"reader should consume exactly what writer wrote"
|
||||
);
|
||||
out
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn read_plain_domain() {
|
||||
// [3]www[6]google[3]com[0]
|
||||
let wire = b"\x03www\x06google\x03com\x00";
|
||||
assert_eq!(roundtrip(wire), "www.google.com");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn read_label_with_literal_dot_is_escaped() {
|
||||
// fanf2's example: [8]exa.mple[3]com[0] — two labels, first contains 0x2E
|
||||
let wire = b"\x08exa.mple\x03com\x00";
|
||||
assert_eq!(roundtrip(wire), "exa\\.mple.com");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn read_label_with_backslash_is_escaped() {
|
||||
// [4]a\bc[3]com[0]
|
||||
let wire = b"\x04a\\bc\x03com\x00";
|
||||
assert_eq!(roundtrip(wire), "a\\\\bc.com");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn read_label_with_nonprintable_byte_uses_decimal_escape() {
|
||||
// [4]\x00foo[3]com[0] — null byte at label start
|
||||
let wire = b"\x04\x00foo\x03com\x00";
|
||||
assert_eq!(roundtrip(wire), "\\000foo.com");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn read_label_with_space_uses_decimal_escape() {
|
||||
// Space (0x20) is outside 0x21..=0x7E, so it must be decimal-escaped.
|
||||
let wire = b"\x05a b c\x00";
|
||||
assert_eq!(roundtrip(wire), "a\\032b\\032c");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_plain_domain() {
|
||||
let mut buf = BytePacketBuffer::new();
|
||||
buf.write_qname("www.google.com").unwrap();
|
||||
assert_eq!(&buf.buf[..buf.pos], b"\x03www\x06google\x03com\x00");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_escaped_dot_does_not_split_label() {
|
||||
let mut buf = BytePacketBuffer::new();
|
||||
buf.write_qname("exa\\.mple.com").unwrap();
|
||||
assert_eq!(&buf.buf[..buf.pos], b"\x08exa.mple\x03com\x00");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_escaped_backslash() {
|
||||
let mut buf = BytePacketBuffer::new();
|
||||
buf.write_qname("a\\\\bc.com").unwrap();
|
||||
assert_eq!(&buf.buf[..buf.pos], b"\x04a\\bc\x03com\x00");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_decimal_escape_yields_raw_byte() {
|
||||
let mut buf = BytePacketBuffer::new();
|
||||
buf.write_qname("\\000foo.com").unwrap();
|
||||
assert_eq!(&buf.buf[..buf.pos], b"\x04\x00foo\x03com\x00");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_rejects_out_of_range_decimal_escape() {
|
||||
let mut buf = BytePacketBuffer::new();
|
||||
assert!(buf.write_qname("\\999foo.com").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_rejects_trailing_backslash() {
|
||||
let mut buf = BytePacketBuffer::new();
|
||||
assert!(buf.write_qname("foo\\").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_rejects_short_decimal_escape() {
|
||||
let mut buf = BytePacketBuffer::new();
|
||||
assert!(buf.write_qname("\\1").is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn roundtrip_preserves_dot_in_label() {
|
||||
assert_eq!(write_then_read("exa\\.mple.com"), "exa\\.mple.com");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn roundtrip_preserves_backslash_in_label() {
|
||||
assert_eq!(write_then_read("a\\\\b.com"), "a\\\\b.com");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn roundtrip_preserves_nonprintable_byte() {
|
||||
assert_eq!(write_then_read("\\000foo.com"), "\\000foo.com");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn root_name_empty_and_dot_both_produce_single_zero() {
|
||||
let mut a = BytePacketBuffer::new();
|
||||
a.write_qname("").unwrap();
|
||||
let mut b = BytePacketBuffer::new();
|
||||
b.write_qname(".").unwrap();
|
||||
assert_eq!(&a.buf[..a.pos], b"\x00");
|
||||
assert_eq!(&b.buf[..b.pos], b"\x00");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user