From 6cc57f7269aa7e1db562dbf648620f9c8ee8a495 Mon Sep 17 00:00:00 2001 From: Jeff Martin Date: Thu, 18 Jul 2024 12:23:04 -0400 Subject: [PATCH] Check NLA buffers on creation Replace calls to `NlaBuffer::new` with `NlaBuffer::new_checked`. This ensures that the parsing of individual NLAs is guaranteed to have a valid NlaBuffer (i.e. sufficient bytes to attempt parsing). Without this check, the parsing logic of several attributes would panic if provided with an NlaBuffer that was too short (e.g. length of 1). One such example is the parsing of `LinkVfPort`, which would have panicked when calling `buf.value()` on line 61 of src/link/sriov/vf_port.rs. --- src/link/af_spec/unspec.rs | 18 +++-- src/link/attribute.rs | 126 +++++++++++++++++++++---------- src/link/sriov/vf_list.rs | 4 +- src/link/sriov/vf_port.rs | 4 +- src/neighbour_table/attribute.rs | 14 +++- 5 files changed, 114 insertions(+), 52 deletions(-) diff --git a/src/link/af_spec/unspec.rs b/src/link/af_spec/unspec.rs index 439d3626..57de4f85 100644 --- a/src/link/af_spec/unspec.rs +++ b/src/link/af_spec/unspec.rs @@ -56,16 +56,22 @@ impl<'a, T: AsRef<[u8]> + ?Sized> Parseable> nlas.push(match nla.kind() { k if k == u8::from(AddressFamily::Inet) as u16 => { AfSpecUnspec::Inet( - VecAfSpecInet::parse(&NlaBuffer::new(&nla.value())) - .context(err)? - .0, + VecAfSpecInet::parse( + &NlaBuffer::new_checked(&nla.value()) + .context(err)?, + ) + .context(err)? + .0, ) } k if k == u8::from(AddressFamily::Inet6) as u16 => { AfSpecUnspec::Inet6( - VecAfSpecInet6::parse(&NlaBuffer::new(&nla.value())) - .context(err)? - .0, + VecAfSpecInet6::parse( + &NlaBuffer::new_checked(&nla.value()) + .context(err)?, + ) + .context(err)? + .0, ) } kind => AfSpecUnspec::Other(DefaultNla::parse(&nla).context( diff --git a/src/link/attribute.rs b/src/link/attribute.rs index 1a7665a3..d33444b1 100644 --- a/src/link/attribute.rs +++ b/src/link/attribute.rs @@ -373,20 +373,41 @@ impl<'a, T: AsRef<[u8]> + ?Sized> ) -> Result { let payload = buf.value(); Ok(match buf.kind() { - IFLA_VFINFO_LIST => Self::VfInfoList( - VecLinkVfInfo::parse(&NlaBuffer::new(payload)) - .context(format!("invalid IFLA_VFINFO_LIST {payload:?}"))? + IFLA_VFINFO_LIST => { + let err = + |payload| format!("invalid IFLA_VFINFO_LIST {payload:?}"); + Self::VfInfoList( + VecLinkVfInfo::parse( + &NlaBuffer::new_checked(payload) + .context(err(payload))?, + ) + .context(err(payload))? .0, - ), - IFLA_VF_PORTS => Self::VfPorts( - VecLinkVfPort::parse(&NlaBuffer::new(payload)) - .context(format!("invalid IFLA_VF_PORTS {payload:?}"))? + ) + } + IFLA_VF_PORTS => { + let err = + |payload| format!("invalid IFLA_VF_PORTS {payload:?}"); + Self::VfPorts( + VecLinkVfPort::parse( + &NlaBuffer::new_checked(payload) + .context(err(payload))?, + ) + .context(err(payload))? .0, - ), - IFLA_PORT_SELF => Self::PortSelf( - LinkVfPort::parse(&NlaBuffer::new(payload)) - .context(format!("invalid IFLA_PORT_SELF {payload:?}"))?, - ), + ) + } + IFLA_PORT_SELF => { + let err = + |payload| format!("invalid IFLA_PORT_SELF {payload:?}"); + Self::PortSelf( + LinkVfPort::parse( + &NlaBuffer::new_checked(payload) + .context(err(payload))?, + ) + .context(err(payload))?, + ) + } IFLA_PHYS_PORT_ID => { Self::PhysPortId(LinkPhysId::parse(payload).context( format!("invalid IFLA_PHYS_PORT_ID value {payload:?}"), @@ -401,29 +422,37 @@ impl<'a, T: AsRef<[u8]> + ?Sized> LinkWirelessEvent::parse(payload) .context(format!("invalid IFLA_WIRELESS {payload:?}"))?, ), - IFLA_PROTINFO => match interface_family { - AddressFamily::Inet6 => Self::ProtoInfoInet6( - VecLinkProtoInfoInet6::parse(&NlaBuffer::new(payload)) - .context(format!( - "invalid IFLA_PROTINFO for AF_INET6 {payload:?}" - ))? + IFLA_PROTINFO => { + let err = |payload| { + format!("invalid IFLA_PROTINFO for AF_INET6 {payload:?}") + }; + match interface_family { + AddressFamily::Inet6 => Self::ProtoInfoInet6( + VecLinkProtoInfoInet6::parse( + &NlaBuffer::new_checked(payload) + .context(err(payload))?, + ) + .context(err(payload))? .0, - ), - #[cfg(any(target_os = "linux", target_os = "fuchsia",))] - AddressFamily::Bridge => Self::ProtoInfoBridge( - VecLinkProtoInfoBridge::parse(&NlaBuffer::new(payload)) + ), + #[cfg(any(target_os = "linux", target_os = "fuchsia",))] + AddressFamily::Bridge => Self::ProtoInfoBridge( + VecLinkProtoInfoBridge::parse(&NlaBuffer::new_checked( + payload, + )?) .context(format!( "invalid IFLA_PROTINFO for AF_INET6 {payload:?}" ))? .0, - ), - _ => Self::ProtoInfoUnknown(DefaultNla::parse(buf).context( - format!( - "invalid IFLA_PROTINFO for \ + ), + _ => Self::ProtoInfoUnknown( + DefaultNla::parse(buf).context(format!( + "invalid IFLA_PROTINFO for \ {interface_family:?}: {payload:?}" + ))?, ), - )?), - }, + } + } IFLA_EVENT => Self::Event( LinkEvent::parse(payload) .context(format!("invalid IFLA_EVENT {payload:?}"))?, @@ -597,24 +626,41 @@ impl<'a, T: AsRef<[u8]> + ?Sized> ) } IFLA_AF_SPEC => match interface_family { - AddressFamily::Unspec => Self::AfSpecUnspec( - VecAfSpecUnspec::parse(&NlaBuffer::new(&buf.value())) - .context("invalid IFLA_AF_SPEC value for AF_UNSPEC")? + AddressFamily::Unspec => { + let err = "invalid IFLA_AF_SPEC value for AF_UNSPEC"; + Self::AfSpecUnspec( + VecAfSpecUnspec::parse( + &NlaBuffer::new_checked(&buf.value()) + .context(err)?, + ) + .context(err)? .0, - ), + ) + } #[cfg(any(target_os = "linux", target_os = "fuchsia",))] - AddressFamily::Bridge => Self::AfSpecBridge( - VecAfSpecBridge::parse(&NlaBuffer::new(&buf.value())) - .context("invalid IFLA_AF_SPEC value for AF_BRIDGE")? + AddressFamily::Bridge => { + let err = "invalid IFLA_AF_SPEC value for AF_BRIDGE"; + Self::AfSpecBridge( + VecAfSpecBridge::parse( + &NlaBuffer::new_checked(&buf.value()) + .context(err)?, + ) + .context(err)? .0, - ), + ) + } _ => Self::AfSpecUnknown(payload.to_vec()), }, - IFLA_LINKINFO => Self::LinkInfo( - VecLinkInfo::parse(&NlaBuffer::new(&buf.value())) - .context("invalid IFLA_LINKINFO value")? + IFLA_LINKINFO => { + let err = "invalid IFLA_LINKINFO value"; + Self::LinkInfo( + VecLinkInfo::parse( + &NlaBuffer::new_checked(&buf.value()).context(err)?, + ) + .context(err)? .0, - ), + ) + } IFLA_XDP => { let err = "invalid IFLA_XDP value"; let buf = NlaBuffer::new_checked(payload).context(err)?; diff --git a/src/link/sriov/vf_list.rs b/src/link/sriov/vf_list.rs index cd5c631e..4d17f851 100644 --- a/src/link/sriov/vf_list.rs +++ b/src/link/sriov/vf_list.rs @@ -31,7 +31,9 @@ impl<'a, T: AsRef<[u8]> + ?Sized> Parseable> buf.value() ))?; if nla.kind() == IFLA_VF_INFO { - nlas.push(LinkVfInfo::parse(&NlaBuffer::new(nla.value()))?); + nlas.push(LinkVfInfo::parse(&NlaBuffer::new_checked( + nla.value(), + )?)?); } else { log::warn!( "BUG: Expecting IFLA_VF_INFO in IFLA_VFINFO_LIST, \ diff --git a/src/link/sriov/vf_port.rs b/src/link/sriov/vf_port.rs index ac759e2f..a63d1935 100644 --- a/src/link/sriov/vf_port.rs +++ b/src/link/sriov/vf_port.rs @@ -20,7 +20,9 @@ impl<'a, T: AsRef<[u8]> + ?Sized> Parseable> buf.value() ))?; if nla.kind() == IFLA_VF_PORT { - nlas.push(LinkVfPort::parse(&NlaBuffer::new(nla.value()))?); + nlas.push(LinkVfPort::parse(&NlaBuffer::new_checked( + nla.value(), + )?)?); } else { log::warn!( "BUG: Expecting IFLA_VF_PORT in IFLA_VF_PORTS, \ diff --git a/src/neighbour_table/attribute.rs b/src/neighbour_table/attribute.rs index 45326e26..43524bb1 100644 --- a/src/neighbour_table/attribute.rs +++ b/src/neighbour_table/attribute.rs @@ -110,11 +110,17 @@ impl<'a, T: AsRef<[u8]> + ?Sized> Parseable> ) .context(format!("invalid NDTA_STATS {payload:?}"))?, ), - NDTA_PARMS => Self::Parms( - VecNeighbourTableParameter::parse(&NlaBuffer::new(payload)) - .context(format!("invalid NDTA_PARMS {payload:?}"))? + NDTA_PARMS => { + let err = |payload| format!("invalid NDTA_PARMS {payload:?}"); + Self::Parms( + VecNeighbourTableParameter::parse( + &NlaBuffer::new_checked(payload) + .context(err(payload))?, + ) + .context(err(payload))? .0, - ), + ) + } NDTA_GC_INTERVAL => Self::GcInterval( parse_u64(payload).context("invalid NDTA_GC_INTERVAL value")?, ),