Merge #283: Backport #271: integer overflow checks

34aa968 bump version to 0.3.1 (Andrew Poelstra)
7f03b7e test: add unit test which causes addition overflow in type size computation (Andrew Poelstra)
e439bc0 bit machine: check resource bounds on construction (Andrew Poelstra)
4f147ca types: use saturating addition in bit width computation during type construction (Andrew Poelstra)

Pull request description:

  Backports several integer overflow checks to the currently-released version of 0.3. These can be used to crash rust-simplicity which is making it hard to write regression fuzztests.

  Also changes the version to 0.3.1 so we can release immediately.

  Should be easy enough to review with `git range-diff pr/271/head...pr/283/head`.

ACKs for top commit:
  uncomputable:
    ACK 34aa968

Tree-SHA512: 2df90cb26d0cbe50e15381c1b8368ca394ee592937f0f173068d2b891bec138e7e23aefcb19c58c51868e373c7d35f7b09930afcc31ebfb86f859fb2f54db657

Author: apoelstra

Cargo-recent.lock

@@ -435,7 +435,7 @@ dependencies = [
 
 [[package]]
 name = "simplicity-lang"
-version = "0.3.0"
+version = "0.3.1"
 dependencies = [
  "bitcoin",
  "bitcoin_hashes",

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "simplicity-lang"
-version = "0.3.0"
+version = "0.3.1"
 authors = ["Andrew Poelstra <[email protected]>"]
 license = "CC0-1.0"
 homepage = "https://github.com/BlockstreamResearch/rust-simplicity/"

src/bit_machine/limits.rs

@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: CC0-1.0
+
+//! Bit Machine Resource Limits
+//!
+//! Implementation of the Bit Machine, without TCO, as TCO precludes some
+//! frame management optimizations which can be used to great benefit.
+//!
+
+use core::fmt;
+use std::error;
+
+/// The maximum number of cells needed by the bit machine.
+///
+/// This roughly corresponds to the maximum size of any type used by a
+/// program, in bits. In a blockchain context this should be limited by
+/// the transaction's weight budget.
+///
+/// The limit here is an absolute limit enforced by the library to avoid
+/// unbounded allocations.
+// This value must be less than usize::MAX / 2 to avoid panics in this module.
+const MAX_CELLS: usize = 2 * 1024 * 1024 * 1024 - 1;
+
+/// The maximum number of frames needed by the bit machine.
+///
+/// This roughly corresponds to the maximum depth of nested `comp` and
+/// `disconnect` combinators. In a blockchain context this should be
+/// limited by the transaction's weight budget.
+///
+/// The limit here is an absolute limit enforced by the library to avoid
+/// unbounded allocations.
+// This value must be less than usize::MAX / 2 to avoid panics in this module.
+const MAX_FRAMES: usize = 1024 * 1024;
+
+#[non_exhaustive]
+#[derive(Clone, Debug)]
+pub enum LimitError {
+    MaxCellsExceeded {
+        /// The number of cells needed by the program.
+        got: usize,
+        /// The maximum allowed number of cells.
+        max: usize,
+        /// A description of which cell count exceeded the limit.
+        bound: &'static str,
+    },
+    MaxFramesExceeded {
+        /// The number of frames needed by the program.
+        got: usize,
+        /// The maximum allowed number of frames.
+        max: usize,
+        /// A description of which frame count exceeded the limit.
+        bound: &'static str,
+    },
+}
+
+impl fmt::Display for LimitError {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        let (limit, got, max, bound) = match self {
+            LimitError::MaxCellsExceeded { got, max, bound } => ("cells", got, max, bound),
+            LimitError::MaxFramesExceeded { got, max, bound } => ("frames", got, max, bound),
+        };
+        write!(
+            f,
+            "maximum number of {} exceeded (needed {}, maximum {}) (bound: {})",
+            limit, got, max, bound,
+        )
+    }
+}
+impl error::Error for LimitError {}
+
+impl LimitError {
+    fn check_max_cells(got: usize, bound: &'static str) -> Result<(), Self> {
+        if got > MAX_CELLS {
+            Err(Self::MaxCellsExceeded {
+                got,
+                max: MAX_CELLS,
+                bound,
+            })
+        } else {
+            Ok(())
+        }
+    }
+
+    fn check_max_frames(got: usize, bound: &'static str) -> Result<(), Self> {
+        if got > MAX_FRAMES {
+            Err(Self::MaxFramesExceeded {
+                got,
+                max: MAX_CELLS,
+                bound,
+            })
+        } else {
+            Ok(())
+        }
+    }
+
+    /// Helper function to check every value and sum for being within bounds.
+    pub(super) fn check_program<J: crate::jet::Jet>(
+        program: &crate::RedeemNode<J>,
+    ) -> Result<(), Self> {
+        let source_ty_width = program.arrow().source.bit_width();
+        let target_ty_width = program.arrow().target.bit_width();
+        let bounds = program.bounds();
+
+        Self::check_max_cells(source_ty_width, "source type width")?;
+        Self::check_max_cells(target_ty_width, "target type width")?;
+        Self::check_max_cells(bounds.extra_cells, "extra cells")?;
+        Self::check_max_cells(
+            source_ty_width + target_ty_width,
+            "source + target type widths",
+        )?;
+        Self::check_max_cells(
+            source_ty_width + target_ty_width + bounds.extra_cells,
+            "source + target type widths + extra cells",
+        )?;
+
+        Self::check_max_frames(bounds.extra_frames, "extra frames")?;
+        Self::check_max_frames(
+            bounds.extra_frames + crate::analysis::IO_EXTRA_FRAMES,
+            "extra frames + fixed overhead",
+        )?;
+        Ok(())
+    }
+}

src/bit_machine/mod.rs

@@ -1,12 +1,13 @@
 // SPDX-License-Identifier: CC0-1.0
 
-//! # Simplicity Execution
+//! Simplicity Execution
 //!
 //! Implementation of the Bit Machine, without TCO, as TCO precludes some
 //! frame management optimizations which can be used to great benefit.
 //!
 
 mod frame;
+mod limits;
 
 use std::error;
 use std::fmt;
@@ -19,6 +20,8 @@ use crate::types::Final;
 use crate::{Cmr, FailEntropy, Value};
 use frame::Frame;
 
+pub use self::limits::LimitError;
+
 /// An execution context for a Simplicity program
 pub struct BitMachine {
     /// Space for bytes that read and write frames point to.
@@ -36,16 +39,17 @@ pub struct BitMachine {
 
 impl BitMachine {
     /// Construct a Bit Machine with enough space to execute the given program.
-    pub fn for_program<J: Jet>(program: &RedeemNode<J>) -> Self {
+    pub fn for_program<J: Jet>(program: &RedeemNode<J>) -> Result<Self, LimitError> {
+        LimitError::check_program(program)?;
         let io_width = program.arrow().source.bit_width() + program.arrow().target.bit_width();
 
-        Self {
+        Ok(Self {
             data: vec![0; (io_width + program.bounds().extra_cells + 7) / 8],
             next_frame_start: 0,
             read: Vec::with_capacity(program.bounds().extra_frames + analysis::IO_EXTRA_FRAMES),
             write: Vec::with_capacity(program.bounds().extra_frames + analysis::IO_EXTRA_FRAMES),
             source_ty: program.arrow().source.clone(),
-        }
+        })
     }
 
     #[cfg(test)]
@@ -60,7 +64,7 @@ impl BitMachine {
             .expect("finalizing types")
             .finalize(&mut SimpleFinalizer::new(None.into_iter()))
             .expect("finalizing");
-        let mut mac = BitMachine::for_program(&prog);
+        let mut mac = BitMachine::for_program(&prog).expect("program has reasonable bounds");
         mac.exec(&prog, env)
     }
 
@@ -481,6 +485,8 @@ pub enum ExecutionError {
     ReachedFailNode(FailEntropy),
     /// Reached a pruned branch
     ReachedPrunedBranch(Cmr),
+    /// Exceeded some program limit
+    LimitExceeded(LimitError),
     /// Jet failed during execution
     JetFailed(JetFailed),
 }
@@ -497,12 +503,29 @@ impl fmt::Display for ExecutionError {
             ExecutionError::ReachedPrunedBranch(hash) => {
                 write!(f, "Execution reached a pruned branch: {}", hash)
             }
-            ExecutionError::JetFailed(jet_failed) => fmt::Display::fmt(jet_failed, f),
+            ExecutionError::LimitExceeded(e) => e.fmt(f),
+            ExecutionError::JetFailed(jet_failed) => jet_failed.fmt(f),
         }
     }
 }
 
-impl error::Error for ExecutionError {}
+impl error::Error for ExecutionError {
+    fn source(&self) -> Option<&(dyn error::Error + 'static)> {
+        match self {
+            Self::InputWrongType(..)
+            | Self::ReachedFailNode(..)
+            | Self::ReachedPrunedBranch(..) => None,
+            Self::LimitExceeded(ref e) => Some(e),
+            Self::JetFailed(ref e) => Some(e),
+        }
+    }
+}
+
+impl From<LimitError> for ExecutionError {
+    fn from(e: LimitError) -> Self {
+        ExecutionError::LimitExceeded(e)
+    }
+}
 
 impl From<JetFailed> for ExecutionError {
     fn from(jet_failed: JetFailed) -> Self {
@@ -512,7 +535,6 @@ impl From<JetFailed> for ExecutionError {
 
 #[cfg(test)]
 mod tests {
-    #[cfg(feature = "elements")]
     use super::*;
 
     #[cfg(feature = "elements")]
@@ -568,7 +590,9 @@ mod tests {
 
         // Try to run it on the bit machine and return the result
         let env = ElementsEnv::dummy();
-        BitMachine::for_program(&prog).exec(&prog, &env)
+        BitMachine::for_program(&prog)
+            .expect("program has reasonable bounds")
+            .exec(&prog, &env)
     }
 
     #[test]
@@ -597,4 +621,20 @@ mod tests {
         );
         assert_eq!(res.unwrap(), Value::unit());
     }
+
+    #[test]
+    fn crash_regression2() {
+        use crate::node::{CoreConstructible as _, JetConstructible as _};
+
+        type Node = Arc<crate::ConstructNode<crate::jet::Core>>;
+
+        let mut bomb = Node::jet(
+            &crate::types::Context::new(),
+            crate::jet::Core::Ch8, // arbitrary jet with nonzero output size
+        );
+        for _ in 0..100 {
+            bomb = Node::pair(&bomb, &bomb).unwrap();
+        }
+        let _ = bomb.finalize_types();
+    }
 }

src/human_encoding/mod.rs

@@ -239,7 +239,7 @@ mod tests {
             .expect("Forest is missing expected root")
             .finalize()
             .expect("Failed to finalize");
-        let mut mac = BitMachine::for_program(&program);
+        let mut mac = BitMachine::for_program(&program).expect("program has reasonable bounds");
         mac.exec(&program, env).expect("Failed to run program");
     }
 
@@ -261,7 +261,7 @@ mod tests {
                 return;
             }
         };
-        let mut mac = BitMachine::for_program(&program);
+        let mut mac = BitMachine::for_program(&program).expect("program has reasonable bounds");
         match mac.exec(&program, env) {
             Ok(_) => panic!("Execution is expected to fail"),
             Err(error) => assert_eq!(&error.to_string(), err_msg),

src/human_encoding/parse/mod.rs

@@ -594,7 +594,8 @@ mod tests {
                     .finalize()
                     .expect("finalize");
 
-                let mut mac = BitMachine::for_program(&program);
+                let mut mac =
+                    BitMachine::for_program(&program).expect("program has reasonable bounds");
                 mac.exec(&program, env).expect("execute");
             }
             Err(errs) => {

src/node/commit.rs

@@ -539,7 +539,8 @@ mod tests {
             assert_eq!(counter, 0..98);
 
             // Execute the program to confirm that it worked
-            let mut mac = BitMachine::for_program(&diff1_final);
+            let mut mac =
+                BitMachine::for_program(&diff1_final).expect("program has reasonable bounds");
             mac.exec(&diff1_final, &()).unwrap();
         }
     }

src/policy/satisfy.rs

@@ -318,15 +318,15 @@ mod tests {
         program: Arc<RedeemNode<Elements>>,
         env: &ElementsEnv<Arc<elements::Transaction>>,
     ) {
-        let mut mac = BitMachine::for_program(&program);
+        let mut mac = BitMachine::for_program(&program).unwrap();
         assert!(mac.exec(&program, env).is_ok());
     }
 
     fn execute_unsuccessful(
         program: Arc<RedeemNode<Elements>>,
         env: &ElementsEnv<Arc<elements::Transaction>>,
     ) {
-        let mut mac = BitMachine::for_program(&program);
+        let mut mac = BitMachine::for_program(&program).unwrap();
         assert!(mac.exec(&program, env).is_err());
     }
 

src/policy/serialize.rs

@@ -287,7 +287,10 @@ mod tests {
         let finalized = commit
             .finalize(&mut SimpleFinalizer::new(witness.into_iter()))
             .expect("finalize");
-        let mut mac = BitMachine::for_program(&finalized);
+        let mut mac = match BitMachine::for_program(&finalized) {
+            Ok(mac) => mac,
+            Err(_) => return false,
+        };
 
         match mac.exec(&finalized, env) {
             Ok(output) => output == Value::unit(),

src/types/final_data.rs

@@ -189,18 +189,23 @@ impl Final {
 
     /// Create the sum of the given `left` and `right` types.
     pub fn sum(left: Arc<Self>, right: Arc<Self>) -> Arc<Self> {
+        // Use saturating_add for bitwidths. If the user has overflowed usize, even on a 32-bit
+        // system this means that they have a 4-gigabit type and their program should be rejected
+        // by a sanity check somewhere. However, if we panic here, the user cannot finalize their
+        // program and cannot even tell that this resource limit has been hit before panicking.
         Arc::new(Final {
             tmr: Tmr::sum(left.tmr, right.tmr),
-            bit_width: 1 + cmp::max(left.bit_width, right.bit_width),
+            bit_width: cmp::max(left.bit_width, right.bit_width).saturating_add(1),
             bound: CompleteBound::Sum(left, right),
         })
     }
 
     /// Create the product of the given `left` and `right` types.
     pub fn product(left: Arc<Self>, right: Arc<Self>) -> Arc<Self> {
+        // See comment in `sum` about use of saturating add.
         Arc::new(Final {
             tmr: Tmr::product(left.tmr, right.tmr),
-            bit_width: left.bit_width + right.bit_width,
+            bit_width: left.bit_width.saturating_add(right.bit_width),
             bound: CompleteBound::Product(left, right),
         })
     }