From b90841dc87d60888b934e9998d254f127c89c341 Mon Sep 17 00:00:00 2001 From: Sergii Glushchenko Date: Mon, 18 Oct 2021 16:10:43 +0200 Subject: [PATCH 1/3] Fix potential overflow in FdtWriter::begin_node() Add a check to make sure than node depth does not exceed usize::MAX. Signed-off-by: Sergii Glushchenko --- src/writer.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/writer.rs b/src/writer.rs index c3d0974..f1b09ef 100644 --- a/src/writer.rs +++ b/src/writer.rs @@ -41,6 +41,8 @@ pub enum Error { InvalidNodeName, /// Invalid property name. InvalidPropertyName, + /// Node depth exceeds usize::MAX + NodeDepthTooLarge, } impl fmt::Display for Error { @@ -65,6 +67,7 @@ impl fmt::Display for Error { } Error::InvalidNodeName => write!(f, "Invalid node name"), Error::InvalidPropertyName => write!(f, "Invalid property name"), + Error::NodeDepthTooLarge => write!(f, "Node depth exceeds usize::MAX"), } } } @@ -331,6 +334,10 @@ impl FdtWriter { /// /// `name` - name of the node; must not contain any NUL bytes. pub fn begin_node(&mut self, name: &str) -> Result { + if self.node_depth == usize::MAX { + return Err(Error::NodeDepthTooLarge); + } + let name_cstr = CString::new(name).map_err(|_| Error::InvalidString)?; // The unit adddress part of the node name, if present, is not fully validated // since the exact requirements depend on the bus mapping. @@ -341,6 +348,8 @@ impl FdtWriter { self.append_u32(FDT_BEGIN_NODE); self.data.extend(name_cstr.to_bytes_with_nul()); self.align(4); + // This can not overflow due to the `if` at the beginning of the function + // where the current depth is checked against usize::MAX. self.node_depth += 1; self.node_ended = false; Ok(FdtWriterNode { @@ -1095,4 +1104,11 @@ mod tests { // Name too long. assert!(!property_name_valid("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")); } + + #[test] + fn depth_overflow() { + let mut fdt = FdtWriter::new().unwrap(); + fdt.node_depth = usize::MAX; + assert_eq!(fdt.begin_node("").unwrap_err(), Error::NodeDepthTooLarge); + } } From e39366146ba1a0f08736b839b2b7884433a69222 Mon Sep 17 00:00:00 2001 From: Sergii Glushchenko Date: Tue, 19 Oct 2021 12:02:46 +0200 Subject: [PATCH 2/3] Limit max node depth to 64 Signed-off-by: Sergii Glushchenko --- src/writer.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/writer.rs b/src/writer.rs index f1b09ef..bd95816 100644 --- a/src/writer.rs +++ b/src/writer.rs @@ -41,7 +41,7 @@ pub enum Error { InvalidNodeName, /// Invalid property name. InvalidPropertyName, - /// Node depth exceeds usize::MAX + /// Node depth exceeds FDT_MAX_NODE_DEPTH NodeDepthTooLarge, } @@ -67,7 +67,7 @@ impl fmt::Display for Error { } Error::InvalidNodeName => write!(f, "Invalid node name"), Error::InvalidPropertyName => write!(f, "Invalid property name"), - Error::NodeDepthTooLarge => write!(f, "Node depth exceeds usize::MAX"), + Error::NodeDepthTooLarge => write!(f, "Node depth exceeds FDT_MAX_NODE_DEPTH"), } } } @@ -80,6 +80,7 @@ pub type Result = std::result::Result; const FDT_HEADER_SIZE: usize = 40; const FDT_VERSION: u32 = 17; const FDT_LAST_COMP_VERSION: u32 = 16; +const FDT_MAX_NODE_DEPTH: usize = 64; /// Interface for writing a Flattened Devicetree (FDT) and emitting a Devicetree Blob (DTB). #[derive(Debug)] @@ -334,7 +335,7 @@ impl FdtWriter { /// /// `name` - name of the node; must not contain any NUL bytes. pub fn begin_node(&mut self, name: &str) -> Result { - if self.node_depth == usize::MAX { + if self.node_depth >= FDT_MAX_NODE_DEPTH { return Err(Error::NodeDepthTooLarge); } @@ -349,7 +350,7 @@ impl FdtWriter { self.data.extend(name_cstr.to_bytes_with_nul()); self.align(4); // This can not overflow due to the `if` at the beginning of the function - // where the current depth is checked against usize::MAX. + // where the current depth is checked against FDT_MAX_NODE_DEPTH. self.node_depth += 1; self.node_ended = false; Ok(FdtWriterNode { @@ -1108,7 +1109,12 @@ mod tests { #[test] fn depth_overflow() { let mut fdt = FdtWriter::new().unwrap(); - fdt.node_depth = usize::MAX; - assert_eq!(fdt.begin_node("").unwrap_err(), Error::NodeDepthTooLarge); + for _ in 1..=FDT_MAX_NODE_DEPTH { + fdt.begin_node("test").unwrap(); + } + assert_eq!( + fdt.begin_node("test").unwrap_err(), + Error::NodeDepthTooLarge + ); } } From a568a59e049c6757898186cbfbda461df072cd81 Mon Sep 17 00:00:00 2001 From: Sergii Glushchenko Date: Tue, 19 Oct 2021 13:23:17 +0200 Subject: [PATCH 3/3] Adjust test coverage Signed-off-by: Sergii Glushchenko --- .gitignore | 1 + coverage_config_x86_64.json | 2 +- src/writer.rs | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index a9d37c5..acbde85 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ target Cargo.lock +kcov_output* diff --git a/coverage_config_x86_64.json b/coverage_config_x86_64.json index 04594a7..a298c00 100644 --- a/coverage_config_x86_64.json +++ b/coverage_config_x86_64.json @@ -1,5 +1,5 @@ { - "coverage_score": 88.0, + "coverage_score": 87.8, "exclude_path": "", "crate_features": "long_running_test" } diff --git a/src/writer.rs b/src/writer.rs index bd95816..ca8bee0 100644 --- a/src/writer.rs +++ b/src/writer.rs @@ -80,6 +80,7 @@ pub type Result = std::result::Result; const FDT_HEADER_SIZE: usize = 40; const FDT_VERSION: u32 = 17; const FDT_LAST_COMP_VERSION: u32 = 16; +/// The same max depth as in the Linux kernel. const FDT_MAX_NODE_DEPTH: usize = 64; /// Interface for writing a Flattened Devicetree (FDT) and emitting a Devicetree Blob (DTB).