Executor: bail for very large cycle counts (#886)

Some instructions within the zkVM have variable cycle counts that are
configured by the guest code. A malicious host can alter parameters to
ecalls such as SHA so that a single instruction to this ecall can exceed
the segment cycle limit. If a single instruction takes more cycles than
a single segment, there is no way to split the instruction among
multiple segments. Return an error for this case to avoid infinite
splits.

risc0/cargo-risczero/src/commands/build_guest.rs

@@ -214,7 +214,7 @@ mod test {
         let tester = Tester::new("risc0/zkvm/methods/guest/Cargo.toml");
         tester.compare_image_id(
             "risc0_zkvm_methods_guest/multi_test",
-            "aa593b59f897db9926bf90d3c252f01eb6ed805ea0ab5894ead20e18397fae08",
+            "c094a8253e33eefda14e0a3b96f192ab30d8f35cc640191cf0533e9371c8c74a",
         );
         tester.compare_image_id(
             "risc0_zkvm_methods_guest/hello_commit",

risc0/zkvm/methods/guest/src/bin/multi_test.rs

@@ -233,5 +233,8 @@ pub fn main() {
             // fault
             asm!( "mv x6, {}", "sw x5, (x6)" , in(reg) addr, out("x5") _, out("x6") _);
         },
+        MultiTestSpec::TooManySha => unsafe {
+            asm!("ecall", in("x5") 3, in("x10") 0, in("x11") 0, in("x12") 0, in("x13") 0, in("x14") 10000,);
+        },
     }
 }

risc0/zkvm/methods/src/multi_test.rs

@@ -74,6 +74,7 @@ pub enum MultiTestSpec {
     Oom,
     OutOfBounds,
     RsaCompat,
+    TooManySha,
 }
 
 declare_syscall!(pub SYS_MULTI_TEST);

risc0/zkvm/src/host/server/exec/executor.rs

@@ -302,6 +302,7 @@ impl<'a> Executor<'a> {
                 return Ok(Some(ExitCode::SessionLimit));
             }
         }
+        let pre_cycles = self.total_cycles();
 
         let insn = self.monitor.load_u32(self.pc)?;
         let opcode = OpCode::decode(insn, self.pc)?;
@@ -380,6 +381,12 @@ impl<'a> Executor<'a> {
         //     total_pending_cycles,
         //     self.total_cycles()
         // );
+        if total_pending_cycles - pre_cycles > self.segment_limit {
+            // some instructions could be invoked with parameters that increase the cycle
+            // count over the segment limit. If this is the case, doing a system split won't
+            // do anything so halt the executor.
+            bail!("execution of instruction at pc [0x{:08x}] resulted in a cycle count too large to fit into a single segment.", self.pc);
+        }
         let exit_code = if total_pending_cycles > self.segment_limit {
             self.split_insn = Some(self.insn_counter);
             log::debug!("split: [{}] pc: 0x{:08x}", self.segment_cycle, self.pc,);

risc0/zkvm/src/host/server/exec/tests.rs

@@ -742,3 +742,14 @@ fn post_state_digest_randomization() {
         .collect();
     assert_eq!(post_state_digests.len(), 1);
 }
+
+#[test]
+#[should_panic(expected = "cycle count too large")]
+fn too_many_sha() {
+    let spec = to_vec(&MultiTestSpec::TooManySha).unwrap();
+    let env = ExecutorEnv::builder().add_input(&spec).build().unwrap();
+    Executor::from_elf(env, MULTI_TEST_ELF)
+        .unwrap()
+        .run()
+        .unwrap();
+}