fix(metering): Prevent execution past limit

Inject metering checks ahead of executing the next code block, instead
of after. This ensures that execution will always return with an error
as soon as it is determined that the execution limit will be exceeded,
without ever actually allowing execution to exceed the limit.

If execution returns with ExecutionLimitExceededError, get_points_used
will return some number of points greater than the limit, since the
INTERNAL_FIELD_USED is incremented and saved prior to the limit check.

Fix wasmerio#999

lib/middleware-common-tests/src/lib.rs

@@ -140,7 +140,7 @@ mod tests {
         assert_eq!(value, 7);
 
         // verify it used the correct number of points
-        assert_eq!(get_points_used(&instance), 74);
+        assert_eq!(get_points_used(&instance), 79);
     }
 
     #[test]
@@ -186,6 +186,51 @@ mod tests {
         }
 
         // verify it used the correct number of points
-        assert_eq!(get_points_used(&instance), 109); // Used points will be slightly more than `limit` because of the way we do gas checking.
+        assert_eq!(get_points_used(&instance), 104); // Used points will be slightly more than `limit` because of the way we do gas checking.
+    }
+    #[test]
+    fn test_traps_if_limit_is_short_by_one() {
+        use wasmer_runtime_core::error::RuntimeError;
+        let wasm_binary = wat2wasm(WAT).unwrap();
+
+        let limit = 78u64;
+
+        let (compiler, backend_id) = get_compiler();
+        let module = compile_with(&wasm_binary, &compiler).unwrap();
+
+        let import_object = imports! {};
+        let mut instance = module.instantiate(&import_object).unwrap();
+
+        set_limit(&mut instance, limit);
+        set_points_used(&mut instance, 0u64);
+
+        let add_to: Func<(i32, i32), i32> = instance.func("add_to").unwrap();
+
+        let cv_pushed = if let Some(msm) = instance.module.runnable_module.get_module_state_map() {
+            push_code_version(CodeVersion {
+                baseline: true,
+                msm: msm,
+                base: instance.module.runnable_module.get_code().unwrap().as_ptr() as usize,
+                backend: backend_id,
+            });
+            true
+        } else {
+            false
+        };
+        let result = add_to.call(3, 4);
+        if cv_pushed {
+            pop_code_version().unwrap();
+        }
+
+        let err = result.unwrap_err();
+        match err {
+            RuntimeError::Error { data } => {
+                assert!(data.downcast_ref::<ExecutionLimitExceededError>().is_some());
+            }
+            _ => unreachable!(),
+        }
+
+        // verify it used the correct number of points
+        assert_eq!(get_points_used(&instance), 79); // Used points will be slightly more than `limit` because of the way we do gas checking.
     }
 }

lib/middleware-common/src/metering.rs

@@ -22,12 +22,103 @@ static INTERNAL_FIELD_LIMIT: InternalField = InternalField::allocate();
 /// the same function calls so we can say that the metering is deterministic.
 ///
 pub struct Metering {
-    current_block: u64,
+    cost_operator_idxs: Vec<usize>,
+    current_block_cost: u64,
 }
 
 impl Metering {
     pub fn new() -> Metering {
-        Metering { current_block: 0 }
+        Metering {
+            cost_operator_idxs: Vec::new(),
+            current_block_cost: 0,
+        }
+    }
+
+    /// inject_metering injects a series of opcodes that adds the cost of the next code block to
+    /// INTERNAL_FIELD_USED and then checks if it has exceeded INTERNAL_FIELD_LIMIT. Since the cost
+    /// of the next code block is not known at the point of injection, Operator::Unreachable is
+    /// used in place of Operator::I64Const{COST}, and the position of this Operator in the sink is
+    /// saved, so that it can later be replaced with the correct cost, once it is know later on in
+    /// parsing Events.
+    fn inject_metering<'a, 'b: 'a>(&mut self, sink: &mut EventSink<'a, 'b>) {
+        // PUSH USED
+        sink.push(Event::Internal(InternalEvent::GetInternal(
+            INTERNAL_FIELD_USED.index() as _,
+        )));
+
+        // placeholder for PUSH COST
+        self.cost_operator_idxs.push(sink.buffer.len());
+        sink.push(Event::WasmOwned(Operator::I64Const { value: 0 }));
+
+        // USED + COST
+        sink.push(Event::WasmOwned(Operator::I64Add));
+
+        // SAVE USED
+        sink.push(Event::Internal(InternalEvent::SetInternal(
+            INTERNAL_FIELD_USED.index() as _,
+        )));
+
+        // PUSH USED
+        sink.push(Event::Internal(InternalEvent::GetInternal(
+            INTERNAL_FIELD_USED.index() as _,
+        )));
+
+        // PUSH LIMIT
+        sink.push(Event::Internal(InternalEvent::GetInternal(
+            INTERNAL_FIELD_LIMIT.index() as _,
+        )));
+
+        // IF USED > LIMIT
+        sink.push(Event::WasmOwned(Operator::I64GtU));
+        sink.push(Event::WasmOwned(Operator::If {
+            ty: WpTypeOrFuncType::Type(WpType::EmptyBlockType),
+        }));
+
+        //          TRAP! EXECUTION LIMIT EXCEEDED
+        sink.push(Event::Internal(InternalEvent::Breakpoint(Box::new(|_| {
+            Err(Box::new(ExecutionLimitExceededError))
+        }))));
+
+        // ENDIF
+        sink.push(Event::WasmOwned(Operator::End));
+    }
+
+    fn remove_trailing_injection<'a, 'b: 'a>(&mut self, sink: &mut EventSink<'a, 'b>) {
+        if let Event::WasmOwned(Operator::End) = sink.buffer[sink.buffer.len() - 11] {
+            // Remove the last 10 Operators.
+            sink.buffer.truncate(sink.buffer.len() - 10);
+        }
+    }
+
+    fn set_costs<'a, 'b: 'a>(&mut self, sink: &mut EventSink<'a, 'b>) {
+        for idx in &self.cost_operator_idxs {
+            match sink.buffer[*idx] {
+                Event::WasmOwned(Operator::I64Const { value }) => {
+                    sink.buffer[*idx] = Event::WasmOwned(Operator::I64Const {
+                        value: value + (self.current_block_cost as i64),
+                    });
+                }
+                _ => panic!(),
+            }
+        }
+        self.current_block_cost = 0;
+    }
+
+    fn begin<'a, 'b: 'a>(&mut self, sink: &mut EventSink<'a, 'b>) {
+        self.set_costs(sink);
+        self.inject_metering(sink);
+    }
+    fn end<'a, 'b: 'a>(&mut self, sink: &mut EventSink<'a, 'b>) {
+        self.set_costs(sink);
+        self.cost_operator_idxs.clear();
+    }
+
+    /// increment_cost adds 1 to the current_block_cost.
+    ///
+    /// Later this may be replaced with a cost map for assigning custom unique cost values to
+    /// specific Operators.
+    fn increment_cost<'a, 'b: 'a>(&mut self, _op: &Event<'a, 'b>) {
+        self.current_block_cost += 1;
     }
 }
 
@@ -38,70 +129,62 @@ impl FunctionMiddleware for Metering {
     type Error = String;
     fn feed_event<'a, 'b: 'a>(
         &mut self,
-        op: Event<'a, 'b>,
+        ev: Event<'a, 'b>,
         _module_info: &ModuleInfo,
         sink: &mut EventSink<'a, 'b>,
     ) -> Result<(), Self::Error> {
-        match op {
-            Event::Internal(InternalEvent::FunctionBegin(_)) => {
-                self.current_block = 0;
-            }
+        self.increment_cost(&ev);
+
+        let op_idx;
+        match ev {
+            Event::Internal(ref iev) => match iev {
+                InternalEvent::FunctionBegin(_) => {
+                    sink.push(ev);
+                    self.begin(sink);
+                    return Ok(());
+                }
+                InternalEvent::FunctionEnd => {
+                    self.end(sink);
+                    self.remove_trailing_injection(sink);
+                    sink.push(ev);
+                    return Ok(());
+                }
+                _ => {
+                    sink.push(ev);
+                    return Ok(());
+                }
+            },
             Event::Wasm(&ref op) | Event::WasmOwned(ref op) => {
-                self.current_block += 1;
                 match *op {
-                    Operator::Loop { .. }
-                    | Operator::Block { .. }
-                    | Operator::End
+                    Operator::End
                     | Operator::If { .. }
                     | Operator::Else
-                    | Operator::Unreachable
                     | Operator::Br { .. }
-                    | Operator::BrTable { .. }
                     | Operator::BrIf { .. }
-                    | Operator::Call { .. }
-                    | Operator::CallIndirect { .. }
+                    | Operator::BrTable { .. }
                     | Operator::Return => {
-                        sink.push(Event::Internal(InternalEvent::GetInternal(
-                            INTERNAL_FIELD_USED.index() as _,
-                        )));
-                        sink.push(Event::WasmOwned(Operator::I64Const {
-                            value: self.current_block as i64,
-                        }));
-                        sink.push(Event::WasmOwned(Operator::I64Add));
-                        sink.push(Event::Internal(InternalEvent::SetInternal(
-                            INTERNAL_FIELD_USED.index() as _,
-                        )));
-                        self.current_block = 0;
+                        self.end(sink);
                     }
                     _ => {}
                 }
+
+                op_idx = sink.buffer.len();
+                sink.push(Event::WasmOwned(Operator::Unreachable));
                 match *op {
-                    Operator::Br { .. }
-                    | Operator::BrTable { .. }
-                    | Operator::BrIf { .. }
-                    | Operator::Call { .. }
-                    | Operator::CallIndirect { .. } => {
-                        sink.push(Event::Internal(InternalEvent::GetInternal(
-                            INTERNAL_FIELD_USED.index() as _,
-                        )));
-                        sink.push(Event::Internal(InternalEvent::GetInternal(
-                            INTERNAL_FIELD_LIMIT.index() as _,
-                        )));
-                        sink.push(Event::WasmOwned(Operator::I64GeU));
-                        sink.push(Event::WasmOwned(Operator::If {
-                            ty: WpTypeOrFuncType::Type(WpType::EmptyBlockType),
-                        }));
-                        sink.push(Event::Internal(InternalEvent::Breakpoint(Box::new(|_| {
-                            Err(Box::new(ExecutionLimitExceededError))
-                        }))));
-                        sink.push(Event::WasmOwned(Operator::End));
+                    Operator::Loop { .. }
+                    | Operator::End
+                    | Operator::If { .. }
+                    | Operator::Else
+                    | Operator::BrIf { .. } => {
+                        self.begin(sink);
                     }
                     _ => {}
                 }
             }
-            _ => {}
         }
-        sink.push(op);
+
+        sink.buffer[op_idx] = ev;
+
         Ok(())
     }
 }