From 8cb75abfb5fd1f92dc9c5ea79f39bf3f4ce25005 Mon Sep 17 00:00:00 2001 From: Wouter Ouwens Date: Thu, 17 Sep 2026 11:10:17 +0200 Subject: [PATCH 1/2] Decode nmap-payloads literals instead of harvesting hex digits parser() kept every ASCII hex digit in a payload literal and dropped everything else, with no notion of what was an escape and what was text. `\x06` survived by luck; `public` in the SNMPv1 GetRequest kept only its `b` and `c` and became the single byte 0xbc. The resulting probe is malformed -- its BER header declares an octet string of length 6 and a message length of 31, but 26 bytes go out -- so agents discard it without replying and the port reads as closed. Every payload written as literal text is affected: udp/137, 389, 427, 1900, 3283, 11211 and 626 can never elicit a response. Track quoted regions, decode the escapes, and take every other character as the byte it denotes. Co-Authored-By: Claude Opus 5 (1M context) --- build.rs | 84 ++++++++++++++++++++++++++++++------ tests/udp_payloads.rs | 99 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 171 insertions(+), 12 deletions(-) create mode 100644 tests/udp_payloads.rs diff --git a/build.rs b/build.rs index 63cc0eaf8..5dca3b560 100644 --- a/build.rs +++ b/build.rs @@ -177,30 +177,90 @@ fn payloads_v(fp_map: &BTreeMap) -> BTreeMap> { payb_linenr } -/// Converts a hexadecimal string to a Vec +/// Decodes a payload literal from `nmap-payloads` into the bytes to put on the wire. +/// +/// An entry's payload is one or more double-quoted strings, which the caller has +/// already joined together. Inside a quoted string, `\xNN` and the usual C escapes +/// denote a single byte and every other character stands for itself -- SNMP's +/// community string is written literally as `public`, and SSDP's probe is literal +/// HTTP. Whitespace and the quotes separating concatenated strings are structure, +/// not payload, so they are skipped. /// /// # Arguments /// -/// * `payload` - A string slice containing the hexadecimal payload +/// * `payload` - The joined payload literals, starting just after the first `"` /// /// # Returns /// -/// A vector of bytes representing the decoded payload +/// A vector of the bytes the payload denotes fn parser(payload: &str) -> Vec { - let payload = payload.trim_matches('"'); - let mut tmp_str = String::new(); + let chars: Vec = payload.chars().collect(); let mut bytes: Vec = Vec::new(); + // The caller slices from just past the opening quote, so we start inside one. + let mut in_quotes = true; + let mut i = 0; + + while i < chars.len() { + let char = chars[i]; + + if char == '"' { + in_quotes = !in_quotes; + i += 1; + continue; + } - for (idx, char) in payload.chars().enumerate() { - if char == '\\' && payload.chars().nth(idx + 1) == Some('x') { + if !in_quotes { + i += 1; continue; - } else if char.is_ascii_hexdigit() { - tmp_str.push(char); - if tmp_str.len() == 2 { - bytes.push(u8::from_str_radix(&tmp_str, 16).unwrap()); - tmp_str.clear(); + } + + if char == '\\' && i + 1 < chars.len() { + match chars[i + 1] { + 'x' if i + 3 < chars.len() => { + let hex: String = chars[i + 2..i + 4].iter().collect(); + if let Ok(byte) = u8::from_str_radix(&hex, 16) { + bytes.push(byte); + i += 4; + continue; + } + } + 'n' => { + bytes.push(b'\n'); + i += 2; + continue; + } + 'r' => { + bytes.push(b'\r'); + i += 2; + continue; + } + 't' => { + bytes.push(b'\t'); + i += 2; + continue; + } + '0' => { + bytes.push(0); + i += 2; + continue; + } + '\\' => { + bytes.push(b'\\'); + i += 2; + continue; + } + '"' => { + bytes.push(b'"'); + i += 2; + continue; + } + _ => {} } } + + let mut buf = [0u8; 4]; + bytes.extend_from_slice(char.encode_utf8(&mut buf).as_bytes()); + i += 1; } bytes diff --git a/tests/udp_payloads.rs b/tests/udp_payloads.rs new file mode 100644 index 000000000..83079a70b --- /dev/null +++ b/tests/udp_payloads.rs @@ -0,0 +1,99 @@ +//! The UDP payload table is generated from `nmap-payloads` at build time, so a +//! decoding mistake there is invisible until a scan silently stops finding a +//! protocol. These tests pin the bytes for probes whose payloads are written as +//! literal text rather than `\xNN` escapes, which is where decoding goes wrong. + +use rustscan::generated::get_parsed_data; + +/// Every payload registered for `port`. +fn payloads_for(port: u16) -> Vec<&'static [u8]> { + get_parsed_data() + .iter() + .filter(|(ports, _)| ports.contains(&port)) + .map(|(_, payload)| payload.as_slice()) + .collect() +} + +fn contains(haystack: &[u8], needle: &[u8]) -> bool { + haystack.windows(needle.len()).any(|w| w == needle) +} + +#[test] +fn snmp_probe_carries_the_community_string() { + let payloads = payloads_for(161); + assert!(!payloads.is_empty(), "no payload registered for udp/161"); + + // The v1 GetRequest spells its community out as `public`. Dropping the + // non-hex characters leaves 0xbc, and the BER length prefix then describes + // an octet string longer than the message -- agents discard it in silence. + assert!( + payloads.iter().any(|p| contains(p, b"public")), + "udp/161 payloads lost the community string: {payloads:02x?}" + ); +} + +#[test] +fn snmp_probe_is_well_formed_ber() { + let payload = payloads_for(161) + .into_iter() + .find(|p| contains(p, b"public")) + .expect("no SNMP payload carrying a community string"); + + // SEQUENCE, then a length covering everything after the two-byte header. + assert_eq!(payload[0], 0x30, "SNMP probe must open with a BER SEQUENCE"); + assert_eq!( + usize::from(payload[1]), + payload.len() - 2, + "declared BER length does not match the bytes on the wire" + ); +} + +#[test] +fn netbios_probe_carries_the_wildcard_name() { + let payloads = payloads_for(137); + assert!(!payloads.is_empty(), "no payload registered for udp/137"); + assert!( + payloads + .iter() + .any(|p| contains(p, b"CKAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA")), + "udp/137 payloads lost the encoded wildcard name" + ); +} + +#[test] +fn text_probes_survive_decoding() { + for (port, needle) in [ + (1900u16, b"M-SEARCH * HTTP/1.1".as_slice()), + (389, b"objectClass".as_slice()), + (11211, b"version".as_slice()), + (427, b"service:service-agent".as_slice()), + ] { + let payloads = payloads_for(port); + assert!(!payloads.is_empty(), "no payload registered for udp/{port}"); + assert!( + payloads.iter().any(|p| contains(p, needle)), + "udp/{port} payload lost {:?}", + String::from_utf8_lossy(needle) + ); + } +} + +#[test] +fn escape_sequences_still_decode_to_single_bytes() { + // udp/123's NTP probes are pure `\xNN`, so they pin that the escape path did + // not regress into emitting the literal characters instead. Two variants are + // registered for the port and only one currently survives the port-keyed map, + // so accept either first byte rather than depending on which one wins. + let payloads = payloads_for(123); + assert!(!payloads.is_empty(), "no payload registered for udp/123"); + assert!( + payloads + .iter() + .any(|p| matches!(p.first(), Some(0xE3 | 0xD9))), + "udp/123 payload does not start with an NTP LI/VN/Mode byte: {payloads:02x?}" + ); + assert!( + payloads.iter().all(|p| !contains(p, b"\\x")), + "a payload kept its escape sequence as literal text" + ); +} From c7b2b94830873ab5f3860a933d1ee082519fe9f3 Mon Sep 17 00:00:00 2001 From: Wouter Ouwens Date: Thu, 17 Sep 2026 16:29:54 +0200 Subject: [PATCH 2/2] refactor: simplify UDP payload decoding with a byte iterator --- build.rs | 101 ++++------------------------------------ build/payload.rs | 104 ++++++++++++++++++++++++++++++++++++++++++ tests/udp_payloads.rs | 15 ++++-- 3 files changed, 124 insertions(+), 96 deletions(-) create mode 100644 build/payload.rs diff --git a/build.rs b/build.rs index 5dca3b560..9f5334567 100644 --- a/build.rs +++ b/build.rs @@ -1,3 +1,6 @@ +#[path = "build/payload.rs"] +mod payload_parser; + use std::collections::BTreeMap; use std::fs::{self, File}; @@ -167,105 +170,17 @@ fn payloads_v(fp_map: &BTreeMap) -> BTreeMap> { let mut payb_linenr: BTreeMap> = BTreeMap::new(); for (&line_nr, data) in fp_map { - if data.contains('\"') { - let start = data.find('\"').expect("payload opening \" not found"); - let payloads = &data[start + 1..]; - payb_linenr.insert(line_nr, parser(payloads.trim())); + if let Some(start) = data.find('"') { + let payload = payload_parser::decode_payload(&data[start..]).unwrap_or_else(|error| { + panic!("Invalid nmap-payloads entry {}: {}", line_nr, error) + }); + payb_linenr.insert(line_nr, payload); } } payb_linenr } -/// Decodes a payload literal from `nmap-payloads` into the bytes to put on the wire. -/// -/// An entry's payload is one or more double-quoted strings, which the caller has -/// already joined together. Inside a quoted string, `\xNN` and the usual C escapes -/// denote a single byte and every other character stands for itself -- SNMP's -/// community string is written literally as `public`, and SSDP's probe is literal -/// HTTP. Whitespace and the quotes separating concatenated strings are structure, -/// not payload, so they are skipped. -/// -/// # Arguments -/// -/// * `payload` - The joined payload literals, starting just after the first `"` -/// -/// # Returns -/// -/// A vector of the bytes the payload denotes -fn parser(payload: &str) -> Vec { - let chars: Vec = payload.chars().collect(); - let mut bytes: Vec = Vec::new(); - // The caller slices from just past the opening quote, so we start inside one. - let mut in_quotes = true; - let mut i = 0; - - while i < chars.len() { - let char = chars[i]; - - if char == '"' { - in_quotes = !in_quotes; - i += 1; - continue; - } - - if !in_quotes { - i += 1; - continue; - } - - if char == '\\' && i + 1 < chars.len() { - match chars[i + 1] { - 'x' if i + 3 < chars.len() => { - let hex: String = chars[i + 2..i + 4].iter().collect(); - if let Ok(byte) = u8::from_str_radix(&hex, 16) { - bytes.push(byte); - i += 4; - continue; - } - } - 'n' => { - bytes.push(b'\n'); - i += 2; - continue; - } - 'r' => { - bytes.push(b'\r'); - i += 2; - continue; - } - 't' => { - bytes.push(b'\t'); - i += 2; - continue; - } - '0' => { - bytes.push(0); - i += 2; - continue; - } - '\\' => { - bytes.push(b'\\'); - i += 2; - continue; - } - '"' => { - bytes.push(b'"'); - i += 2; - continue; - } - _ => {} - } - } - - let mut buf = [0u8; 4]; - bytes.extend_from_slice(char.encode_utf8(&mut buf).as_bytes()); - i += 1; - } - - bytes -} - /// Combines the ports BTreeMap and the Payloads BTreeMap /// /// # Arguments diff --git a/build/payload.rs b/build/payload.rs new file mode 100644 index 000000000..a5d4a35a4 --- /dev/null +++ b/build/payload.rs @@ -0,0 +1,104 @@ +/// Decode consecutive quoted strings, stopping before optional entry metadata. +pub fn decode_payload(input: &str) -> Result, &'static str> { + let input = input.trim_start(); + if !input.starts_with('"') { + return Err("expected a quoted payload"); + } + + let mut input = input.bytes().peekable(); + let mut payload = Vec::new(); + + loop { + while input.next_if(u8::is_ascii_whitespace).is_some() {} + if input.next_if_eq(&b'"').is_none() { + return Ok(payload); + } + + loop { + match input.next().ok_or("unterminated payload string")? { + b'"' => break, + b'\\' => payload.push(decode_escape(&mut input)?), + byte => payload.push(byte), + } + } + } +} + +fn decode_escape(input: &mut impl Iterator) -> Result { + match input.next().ok_or("incomplete escape")? { + b'x' => { + let mut byte = 0; + for _ in 0..2 { + let digit = input + .next() + .and_then(|byte| char::from(byte).to_digit(16)) + .ok_or("expected two hex digits after \\x")?; + byte = byte * 16 + digit as u8; + } + Ok(byte) + } + b'n' => Ok(b'\n'), + b'r' => Ok(b'\r'), + b't' => Ok(b'\t'), + b'0' => Ok(0), + b'\\' => Ok(b'\\'), + b'"' => Ok(b'"'), + _ => Err("unsupported escape"), + } +} + +#[cfg(test)] +mod tests { + use super::decode_payload; + + #[test] + fn preserves_literal_bytes_and_decodes_hex_pairs() { + assert_eq!( + decode_payload(r#""\x04\x06public\xa1\xFF0""#).unwrap(), + b"\x04\x06public\xa1\xff0" + ); + } + + #[test] + fn concatenates_strings_without_losing_literal_whitespace() { + assert_eq!( + decode_payload(" \" public \"\n\t\"\"\"name \" ").unwrap(), + b" public name " + ); + } + + #[test] + fn decodes_escapes_without_treating_escaped_quotes_as_delimiters() { + assert_eq!( + decode_payload(r#""\0\r\n\t\\\"public\"""#).unwrap(), + b"\0\r\n\t\\\"public\"" + ); + } + + #[test] + fn stops_before_metadata_even_when_it_contains_quotes() { + assert_eq!( + decode_payload(r#""public" source 161 future "metadata""#).unwrap(), + b"public" + ); + } + + #[test] + fn rejects_malformed_payloads() { + for input in [ + "", + "public", + "\"public", + "\"public\" \"unfinished", + "\"\\", + r#""\q""#, + r#""\x""#, + r#""\x0""#, + r#""\xGG""#, + r#""\x0" "1""#, + r#""public\""#, + ] { + assert!(decode_payload(input).is_err(), "accepted {:?}", input); + } + } +} diff --git a/tests/udp_payloads.rs b/tests/udp_payloads.rs index 83079a70b..358df0677 100644 --- a/tests/udp_payloads.rs +++ b/tests/udp_payloads.rs @@ -3,6 +3,9 @@ //! protocol. These tests pin the bytes for probes whose payloads are written as //! literal text rather than `\xNN` escapes, which is where decoding goes wrong. +#[path = "../build/payload.rs"] +mod payload_parser; + use rustscan::generated::get_parsed_data; /// Every payload registered for `port`. @@ -28,7 +31,8 @@ fn snmp_probe_carries_the_community_string() { // an octet string longer than the message -- agents discard it in silence. assert!( payloads.iter().any(|p| contains(p, b"public")), - "udp/161 payloads lost the community string: {payloads:02x?}" + "udp/161 payloads lost the community string: {:02x?}", + payloads ); } @@ -69,7 +73,11 @@ fn text_probes_survive_decoding() { (427, b"service:service-agent".as_slice()), ] { let payloads = payloads_for(port); - assert!(!payloads.is_empty(), "no payload registered for udp/{port}"); + assert!( + !payloads.is_empty(), + "no payload registered for udp/{}", + port + ); assert!( payloads.iter().any(|p| contains(p, needle)), "udp/{port} payload lost {:?}", @@ -90,7 +98,8 @@ fn escape_sequences_still_decode_to_single_bytes() { payloads .iter() .any(|p| matches!(p.first(), Some(0xE3 | 0xD9))), - "udp/123 payload does not start with an NTP LI/VN/Mode byte: {payloads:02x?}" + "udp/123 payload does not start with an NTP LI/VN/Mode byte: {:02x?}", + payloads ); assert!( payloads.iter().all(|p| !contains(p, b"\\x")),