From f7dfc0ef6eac7e68818be1c1eb08caa8b56a85bf Mon Sep 17 00:00:00 2001 From: dodic Date: Fri, 1 Jul 2022 21:33:58 +0300 Subject: [PATCH] Fix UB in `Link` and `Link` logic --- rust/doublets/src/data/link.rs | 34 +++++++++++++--------------- rust/doublets/src/mem/split/store.rs | 16 ++++++++----- rust/doublets/src/mem/unit/store.rs | 16 ++++++++----- rust/doublets/tests/link.rs | 8 +++++++ 4 files changed, 44 insertions(+), 30 deletions(-) create mode 100644 rust/doublets/tests/link.rs diff --git a/rust/doublets/src/data/link.rs b/rust/doublets/src/data/link.rs index 2adab80d9..d4510dc7d 100644 --- a/rust/doublets/src/data/link.rs +++ b/rust/doublets/src/data/link.rs @@ -1,15 +1,17 @@ -use std::default::default; -use std::fmt::{Debug, Display, Formatter}; -use std::ops::Index; -use std::slice::{from_raw_parts, SliceIndex}; +use std::{ + fmt, + fmt::{Debug, Formatter}, + ops::Index, + slice::{from_raw_parts, SliceIndex}, +}; use num_traits::zero; -use data::Query; -use data::ToQuery; +use data::{Query, ToQuery}; use num::LinkType; -#[derive(Default, Debug, Eq, PartialEq, Clone, Hash)] +#[derive(Default, Eq, PartialEq, Clone, Hash)] +#[repr(C)] pub struct Link { pub index: T, pub source: T, @@ -18,7 +20,7 @@ pub struct Link { impl Link { pub fn nothing() -> Self { - default() + Self::default() } pub fn new(index: T, source: T, target: T) -> Self { @@ -44,25 +46,21 @@ impl Link { pub fn is_partial(&self) -> bool { self.index == self.source || self.index == self.target } -} - -impl> Index for Link { - type Output = I::Output; - fn index(&self, index: I) -> &Self::Output { - let slice = unsafe { from_raw_parts(&self.index, 3) }; - Index::index(slice, index) + pub fn as_slice(&self) -> &[T] { + // SAFETY: Link is repr(C) and therefore is safe to transmute to a slice + unsafe { from_raw_parts(&self.index, 3) } } } -impl Display for Link { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { +impl Debug for Link { + fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { write!(f, "({}: {} {})", self.index, self.source, self.target) } } impl ToQuery for Link { fn to_query(&self) -> Query<'_, T> { - Query::new(unsafe { from_raw_parts(&self.index, 3) }) + self.as_slice().to_query() } } diff --git a/rust/doublets/src/mem/split/store.rs b/rust/doublets/src/mem/split/store.rs index 70c7fdb44..20bf416c0 100644 --- a/rust/doublets/src/mem/split/store.rs +++ b/rust/doublets/src/mem/split/store.rs @@ -810,12 +810,14 @@ impl< query: &[T], handler: WriteHandler, ) -> Result> { - self.create_by_with(query, |before, after| handler(&before[..], &after[..])) - .map_err(|err| err.into()) + self.create_by_with(query, |before, after| { + handler(before.as_slice(), after.as_slice()) + }) + .map_err(|err| err.into()) } fn each_links(&self, query: &[T], handler: ReadHandler) -> Result> { - Ok(self.each_by(query, |link| handler(&link[..]))) + Ok(self.each_by(query, |link| handler(link.as_slice()))) } fn update_links( @@ -825,7 +827,7 @@ impl< handler: WriteHandler, ) -> Result> { self.update_by_with(query, change, |before, after| { - handler(&before[..], &after[..]) + handler(before.as_slice(), after.as_slice()) }) .map_err(|err| err.into()) } @@ -835,8 +837,10 @@ impl< query: &[T], handler: WriteHandler, ) -> Result> { - self.delete_by_with(query, |before, after| handler(&before[..], &after[..])) - .map_err(|err| err.into()) + self.delete_by_with(query, |before, after| { + handler(before.as_slice(), after.as_slice()) + }) + .map_err(|err| err.into()) } } diff --git a/rust/doublets/src/mem/unit/store.rs b/rust/doublets/src/mem/unit/store.rs index 48e3012f3..67f8974f8 100644 --- a/rust/doublets/src/mem/unit/store.rs +++ b/rust/doublets/src/mem/unit/store.rs @@ -603,12 +603,14 @@ impl>, TS: UnitTree, TT: UnitTree, TU: query: &[T], handler: WriteHandler, ) -> Result> { - self.create_by_with(query, |before, after| handler(&before[..], &after[..])) - .map_err(|err| err.into()) + self.create_by_with(query, |before, after| { + handler(before.as_slice(), after.as_slice()) + }) + .map_err(|err| err.into()) } fn each_links(&self, query: &[T], handler: ReadHandler) -> Result> { - Ok(self.each_by(query, |link| handler(&link[..]))) + Ok(self.each_by(query, |link| handler(link.as_slice()))) } fn update_links( @@ -618,7 +620,7 @@ impl>, TS: UnitTree, TT: UnitTree, TU: handler: WriteHandler, ) -> Result> { self.update_by_with(query, change, |before, after| { - handler(&before[..], &after[..]) + handler(before.as_slice(), after.as_slice()) }) .map_err(|err| err.into()) } @@ -628,8 +630,10 @@ impl>, TS: UnitTree, TT: UnitTree, TU: query: &[T], handler: WriteHandler, ) -> Result> { - self.delete_by_with(query, |before, after| handler(&before[..], &after[..])) - .map_err(|err| err.into()) + self.delete_by_with(query, |before, after| { + handler(before.as_slice(), after.as_slice()) + }) + .map_err(|err| err.into()) } } diff --git a/rust/doublets/tests/link.rs b/rust/doublets/tests/link.rs new file mode 100644 index 000000000..88e9f5cca --- /dev/null +++ b/rust/doublets/tests/link.rs @@ -0,0 +1,8 @@ +use data::ToQuery; +use doublets::Link; + +#[test] +fn link_to_query() { + let link = Link::::new(1, 2, 3); + assert_eq!([1, 2, 3], &link.to_query()[..]); +}