Merge pull request #346 from dusk-network/under-overflow-343

Fix audit-reported under/overflows

piecrust/CHANGELOG.md

@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Add `Error::ArgumentBufferOverflow` variant [#343]
+
+### Fixed
+
+- Fix possible under/overflows reported by audit [#343]
+
 
 ### Changed
 
@@ -392,6 +400,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 [#347]: https://github.com/dusk-network/piecrust/issues/347
 [#344]: https://github.com/dusk-network/piecrust/issues/344
+[#343]: https://github.com/dusk-network/piecrust/issues/343
 [#336]: https://github.com/dusk-network/piecrust/issues/336
 [#325]: https://github.com/dusk-network/piecrust/issues/325
 [#324]: https://github.com/dusk-network/piecrust/issues/324

piecrust/src/call_tree.rs

@@ -150,6 +150,9 @@ impl Drop for CallTree {
 unsafe fn update_spent(node: *mut CallTreeNode) {
     let node = &mut *node;
     node.children.iter_mut().for_each(|&mut child| unsafe {
+        // It should be impossible for this to underflow since the amount spent
+        // in all child nodes is always less than or equal to the amount spent
+        // in the parent node.
         node.elem.spent -= (*child).elem.spent;
         update_spent(child);
     });

piecrust/src/error.rs

@@ -22,6 +22,8 @@ pub type Compo = CompositeSerializerError<
 /// The error type returned by the piecrust VM.
 #[derive(Error, Debug)]
 pub enum Error {
+    #[error("Argument buffer overflow: {len} > {max_len}")]
+    ArgumentBufferOverflow { len: usize, max_len: usize },
     #[error("Commit error: {0}")]
     CommitError(Cow<'static, str>),
     #[error(transparent)]

piecrust/src/imports.rs

@@ -91,7 +91,16 @@ pub fn check_ptr(
 ) -> Result<(), Error> {
     let mem_len = instance.with_memory(|mem| mem.len());
 
-    if offset + len >= mem_len {
+    let end =
+        offset
+            .checked_add(len)
+            .ok_or(Error::MemoryAccessOutOfBounds {
+                offset,
+                len,
+                mem_len,
+            })?;
+
+    if end >= mem_len {
         return Err(Error::MemoryAccessOutOfBounds {
             offset,
             len,

piecrust/src/instance.rs

@@ -263,10 +263,23 @@ impl WrappedInstance {
         })
     }
 
-    pub(crate) fn write_bytes_to_arg_buffer(&mut self, buf: &[u8]) -> u32 {
+    pub(crate) fn write_bytes_to_arg_buffer(
+        &mut self,
+        buf: &[u8],
+    ) -> Result<u32, Error> {
         self.with_arg_buf_mut(|arg_buffer| {
+            if buf.len() > arg_buffer.len() {
+                return Err(Error::MemoryAccessOutOfBounds {
+                    offset: 0,
+                    len: buf.len(),
+                    mem_len: ARGBUF_LEN,
+                });
+            }
+
             arg_buffer[..buf.len()].copy_from_slice(buf);
-            buf.len() as u32
+            // It is safe to cast to u32 because the length of the buffer is
+            // guaranteed to be less than 4GiB.
+            Ok(buf.len() as u32)
         })
     }
 

piecrust/src/session.rs

@@ -761,7 +761,7 @@ impl Session {
                 io: Arc::new(err),
             })?;
 
-        let arg_len = instance.write_bytes_to_arg_buffer(&fdata);
+        let arg_len = instance.write_bytes_to_arg_buffer(&fdata)?;
         let ret_len = instance
             .call(fname, arg_len, limit)
             .map_err(|err| {