diff --git a/nvx-rs/src/protocol/maps.rs b/nvx-rs/src/protocol/maps.rs index 78de1379a..a660e0e7d 100644 --- a/nvx-rs/src/protocol/maps.rs +++ b/nvx-rs/src/protocol/maps.rs @@ -48,7 +48,7 @@ pub fn encode_maps_payloads(maps: &[HostMap], limit: usize) -> Result Result { - let truncated = || violation("the host mapping request is truncated"); let first = u32_at(payload, 0).ok_or_else(truncated)?; let count = u32_at(payload, 4).ok_or_else(truncated)?; let mut maps = Vec::new(); - let mut offset = MAPS_HEADER_LEN; - for _ in 0..count { - let (Some(flags), Some(source_len), Some(target_len)) = ( - u16_at(payload, offset), - u16_at(payload, offset + 2), - u16_at(payload, offset + 4), - ) else { - return Err(truncated()); - }; - offset += MAP_ENTRY_HEADER_LEN; - let mut text = |length: u16| -> Result { - let end = offset + usize::from(length); - let bytes = payload.get(offset..end).ok_or_else(truncated)?; - offset = end; + decode_maps_entries( + payload, + count, + &mut maps, + |bytes| { String::from_utf8(bytes.to_vec()) .map_err(|_| violation("a host mapping is not valid UTF-8")) - }; - let source = text(source_len)?; - let target = text(target_len)?; - if flags & !MAP_READ_ONLY != 0 || !acceptable(&source, &target) { - return Err(violation( - "a host mapping has unknown flags or a source or target that the guest agent \ - refuses", - )); - } - - maps.push(HostMap { + }, + |source, target, read_only| HostMap { source, target, - read_only: flags & MAP_READ_ONLY != 0, - }); - } - if offset != payload.len() { - return Err(violation("the host mapping request has trailing bytes")); - } + read_only, + }, + )?; Ok(MapsPayload { first, maps }) } @@ -170,7 +148,6 @@ pub fn decode_maps_payload(payload: &[u8]) -> Result /// [`decode_maps_payload`] remains UTF-8 for host and fake-OpenVMM callers. This decoder accepts /// every non-NUL Linux path byte that the guest can mount. pub fn decode_guest_maps_payload(payload: &[u8]) -> Result { - let truncated = || violation("the host mapping request is truncated"); let first = u32_at(payload, 0).ok_or_else(truncated)?; let count = u32_at(payload, 4).ok_or_else(truncated)?; let capacity = usize::try_from(count) @@ -178,6 +155,33 @@ pub fn decode_guest_maps_payload(payload: &[u8]) -> Result, M>( + payload: &[u8], + count: u32, + maps: &mut Vec, + path: impl Fn(&[u8]) -> Result, + entry: impl Fn(P, P, bool) -> M, +) -> Result<(), ProtocolError> { let mut offset = MAPS_HEADER_LEN; for _ in 0..count { let (Some(flags), Some(source_len), Some(target_len)) = ( @@ -190,53 +194,40 @@ pub fn decode_guest_maps_payload(payload: &[u8]) -> Result Result, ProtocolError> { + let mut take = |length: u16| -> Result { let end = offset .checked_add(usize::from(length)) .ok_or_else(truncated)?; - let bytes = payload.get(offset..end).ok_or_else(truncated)?.to_vec(); + let bytes = payload.get(offset..end).ok_or_else(truncated)?; offset = end; - Ok(bytes) + path(bytes) }; let source = take(source_len)?; let target = take(target_len)?; - if flags & !MAP_READ_ONLY != 0 || !acceptable_bytes(&source, &target) { + if flags & !MAP_READ_ONLY != 0 || !acceptable(source.as_ref(), target.as_ref()) { return Err(violation( "a host mapping has unknown flags or a source or target that the guest agent \ refuses", )); } - maps.push(GuestHostMap { - source, - target, - read_only: flags & MAP_READ_ONLY != 0, - }); + maps.push(entry(source, target, flags & MAP_READ_ONLY != 0)); } if offset != payload.len() { return Err(violation("the host mapping request has trailing bytes")); } - Ok(GuestMapsPayload { first, maps }) + Ok(()) } -/// Whether the guest agent accepts the paths of a mapping: a relative `source` and an absolute -/// `target` of at most [`MAX_MAP_PATH_BYTES`] without NUL characters, whose `/`-separated -/// components are neither empty, `.`, nor `..`. -fn acceptable(source: &str, target: &str) -> bool { - let normal = |components: &str| { - !components.is_empty() - && components - .split('/') - .all(|component| !matches!(component, "" | "." | "..")) - }; - [source, target] - .iter() - .all(|path| path.len() <= MAX_MAP_PATH_BYTES && !path.contains('\0')) - && normal(source) - && target.strip_prefix('/').is_some_and(normal) +/// Returns the error of a `MAPS` payload that ends before its header or an entry does. +fn truncated() -> ProtocolError { + violation("the host mapping request is truncated") } -/// Applies the guest agent's byte-oriented source and target path rules. -fn acceptable_bytes(source: &[u8], target: &[u8]) -> bool { +/// Whether the guest agent accepts the paths of a mapping: a relative `source` and an absolute +/// `target` of at most [`MAX_MAP_PATH_BYTES`] without NUL characters, whose `/`-separated +/// components are neither empty, `.`, nor `..`. UTF-8 text follows these rules as its bytes, in +/// which `/` and NUL occur only as themselves. +fn acceptable(source: &[u8], target: &[u8]) -> bool { let normal = |components: &[u8]| { !components.is_empty() && components @@ -245,8 +236,7 @@ fn acceptable_bytes(source: &[u8], target: &[u8]) -> bool { }; [source, target] .iter() - .all(|path| !path.is_empty() && path.len() <= MAX_MAP_PATH_BYTES && !path.contains(&0)) - && source != b"." + .all(|path| path.len() <= MAX_MAP_PATH_BYTES && !path.contains(&0)) && normal(source) && target.strip_prefix(b"/").is_some_and(normal) } diff --git a/nvx-rs/src/protocol/tests.rs b/nvx-rs/src/protocol/tests.rs index b96263b89..312071b12 100644 --- a/nvx-rs/src/protocol/tests.rs +++ b/nvx-rs/src/protocol/tests.rs @@ -1317,3 +1317,56 @@ fn guest_maps_decoder_accepts_non_utf8_linux_paths() { let decoded = decode_guest_maps_payload(&payload).unwrap(); assert_eq!(decoded.maps[0].source, b"0/\xff"); } + +/// Both `MAPS` decoders refuse an entry with the same message, except that the host's requires +/// each path to be UTF-8 as soon as it reads it, and the guest's bounds the number of entries +/// before it reads any. +#[test] +fn maps_decoders_refuse_entries_with_the_same_messages() { + let valid = bytes(concat!("0000000001000000", "000001000200", "30", "2f77")); + let mut trailing = valid.clone(); + trailing.push(0); + let mut unknown_flag = valid.clone(); + unknown_flag[8] = 2; + let mut relative_target = valid.clone(); + relative_target[15] = b'w'; + let refused = "a host mapping has unknown flags or a source or target that the guest agent \ + refuses"; + for (payload, expected) in [ + (&valid[..7], "the host mapping request is truncated"), + ( + &valid[..valid.len() - 1], + "the host mapping request is truncated", + ), + (&trailing[..], "the host mapping request has trailing bytes"), + (&unknown_flag[..], refused), + (&relative_target[..], refused), + ] { + assert_eq!( + decode_maps_payload(payload).unwrap_err().message(), + expected + ); + assert_eq!( + decode_guest_maps_payload(payload).unwrap_err().message(), + expected + ); + } + let not_utf8_then_truncated = bytes(concat!("0000000001000000", "000001000200", "ff")); + assert_eq!( + decode_maps_payload(¬_utf8_then_truncated) + .unwrap_err() + .message(), + "a host mapping is not valid UTF-8" + ); + assert_eq!( + decode_guest_maps_payload(¬_utf8_then_truncated) + .unwrap_err() + .message(), + "the host mapping request is truncated" + ); + let too_many = bytes("0000000001100000"); + assert_eq!( + decode_guest_maps_payload(&too_many).unwrap_err().message(), + "the host mapping request has too many entries" + ); +}