Merge #2961

2961: Fuzzing test for syscall exec r=quake,driftluo a=mohanson

<!--
Thank you for contributing to nervosnetwork/ckb!

If you haven't already, please read [CONTRIBUTING](https://github.com/nervosnetwork/ckb/blob/develop/CONTRIBUTING.md) document.

If you're unsure about anything, just ask; somebody should be along to answer within a day or two.

PR Title Format:
1. module [, module2, module3]: what's changed
2. *: what's changed
-->

### What problem does this PR solve?

Problem Summary:

Add fuzz test for exec syscall and fix the problems found in the test. Test results: https://github.com/mohanson/ckb_vm_exec_fuzz/tree/master/v1

### What is changed and how it works?

What's Changed:

- exec always return err code and Ok(true)
- add bound check for exec's offset and  length

### Related changes

- PR to update `owner/repo`:
- Need to cherry-pick to the release branch

### Check List <!--REMOVE the items that are not applicable-->

Tests <!-- At least one of them must be included. -->

- Unit test

### Release note <!-- Choose from None, Title Only and Note. Bugfixes or new features need a release note. -->
None



Co-authored-by: mohanson <[email protected]>

script/fuzz/Cargo.toml

@@ -13,7 +13,8 @@ publish = false
 cargo-fuzz = true
 
 [dependencies]
-libfuzzer-sys = "0.4"
+arbitrary = { version = "1", features = ["derive"] }
+libfuzzer-sys = { version="0.4.0", features=["arbitrary-derive"] }
 ckb-traits = { path = "../../traits", version = "= 0.100.0-pre" }
 ckb-types = { path = "../../util/types", version = "= 0.100.0-pre" }
 ckb-script = { path = "../../script", version = "= 0.100.0-pre" }
@@ -34,3 +35,9 @@ name = "transaction_scripts_verifier_data1"
 path = "fuzz_targets/transaction_scripts_verifier_data1.rs"
 test = false
 doc = false
+
+[[bin]]
+name = "syscall_exec"
+path = "fuzz_targets/syscall_exec.rs"
+test = false
+doc = false

script/fuzz/fuzz_targets/syscall_exec.rs

@@ -0,0 +1,168 @@
+#![no_main]
+use arbitrary::Arbitrary;
+use ckb_chain_spec::consensus::ConsensusBuilder;
+use ckb_script::{TransactionScriptsVerifier, TxVerifyEnv};
+use ckb_traits::{CellDataProvider, HeaderProvider};
+use ckb_types::{
+    bytes::Bytes,
+    core::{
+        capacity_bytes,
+        cell::{CellMetaBuilder, ResolvedTransaction},
+        hardfork::HardForkSwitch,
+        Capacity, HeaderView, ScriptHashType, TransactionBuilder, TransactionInfo,
+    },
+    packed::{
+        Byte32, CellInput, CellOutput, CellOutputBuilder, OutPoint, Script, TransactionInfoBuilder,
+        TransactionKeyBuilder,
+    },
+    prelude::*,
+};
+use libfuzzer_sys::fuzz_target;
+
+#[derive(Default, PartialEq, Eq, Clone)]
+struct MockDataLoader {}
+
+impl CellDataProvider for MockDataLoader {
+    fn get_cell_data(&self, _out_point: &OutPoint) -> Option<Bytes> {
+        None
+    }
+
+    fn get_cell_data_hash(&self, _out_point: &OutPoint) -> Option<Byte32> {
+        None
+    }
+}
+
+impl HeaderProvider for MockDataLoader {
+    fn get_header(&self, _block_hash: &Byte32) -> Option<HeaderView> {
+        None
+    }
+}
+
+fn mock_transaction_info() -> TransactionInfo {
+    TransactionInfoBuilder::default()
+        .block_number(1u64.pack())
+        .block_epoch(0u64.pack())
+        .key(
+            TransactionKeyBuilder::default()
+                .block_hash(Byte32::zero())
+                .index(1u32.pack())
+                .build(),
+        )
+        .build()
+        .unpack()
+}
+
+static CALLER: &[u8] = include_bytes!("../programs/exec_caller");
+static CALLEE: &[u8] = include_bytes!("../programs/exec_callee");
+
+#[derive(Arbitrary, Debug)]
+pub struct FuzzData {
+    from: u32,
+    argv: Vec<String>,
+    callee_data_head: u64,
+    callee_data_tail: u64,
+}
+
+fn run(data: FuzzData) {
+    let exec_caller_cell_data = Bytes::from(CALLER);
+    let exec_callee_cell_data = {
+        let mut r: Vec<u8> = vec![];
+        for _ in 0..data.callee_data_head as u8 {
+            r.push(0x00);
+        }
+        r.extend(CALLEE);
+        for _ in 0..data.callee_data_tail as u8 {
+            r.push(0x00);
+        }
+        Bytes::copy_from_slice(&r)
+    };
+    let exec_caller_data_data = {
+        let mut r: Vec<u8> = vec![];
+        r.push(data.from as u8 % 3);
+        r.push(data.callee_data_head as u8);
+        let l = if data.callee_data_tail as u8 == 0 {
+            0
+        } else {
+            CALLEE.len()
+        };
+        r.extend_from_slice(&l.to_le_bytes());
+        let argc = data.argv.len() as u64;
+        r.extend_from_slice(&argc.to_le_bytes());
+        for i in &data.argv {
+            let l = i.len() as u64 + 1;
+            r.extend_from_slice(&l.to_le_bytes());
+            r.extend_from_slice(i.as_bytes());
+            r.push(0x00);
+        }
+        Bytes::copy_from_slice(&r)
+    };
+
+    let exec_caller_cell = CellOutput::new_builder()
+        .capacity(Capacity::bytes(exec_caller_cell_data.len()).unwrap().pack())
+        .build();
+    let exec_callee_cell = CellOutput::new_builder()
+        .capacity(Capacity::bytes(exec_callee_cell_data.len()).unwrap().pack())
+        .build();
+    let exec_caller_data_cell = CellOutput::new_builder()
+        .capacity(Capacity::bytes(exec_caller_data_data.len()).unwrap().pack())
+        .build();
+
+    let exec_caller_script = Script::new_builder()
+        .hash_type(ScriptHashType::Data1.into())
+        .code_hash(CellOutput::calc_data_hash(&exec_caller_cell_data))
+        .build();
+    let output = CellOutputBuilder::default()
+        .capacity(capacity_bytes!(100).pack())
+        .lock(exec_caller_script)
+        .build();
+    let input = CellInput::new(OutPoint::null(), 0);
+
+    let transaction = TransactionBuilder::default()
+        .input(input)
+        .set_witnesses(vec![exec_callee_cell_data.pack()])
+        .build();
+
+    let dummy_cell = CellMetaBuilder::from_cell_output(output, Bytes::new())
+        .transaction_info(mock_transaction_info())
+        .build();
+    let exec_caller_cell =
+        CellMetaBuilder::from_cell_output(exec_caller_cell, exec_caller_cell_data)
+            .transaction_info(mock_transaction_info())
+            .build();
+
+    let exec_callee_cell =
+        CellMetaBuilder::from_cell_output(exec_callee_cell, exec_callee_cell_data)
+            .transaction_info(mock_transaction_info())
+            .build();
+    let exec_caller_data_cell =
+        CellMetaBuilder::from_cell_output(exec_caller_data_cell, exec_caller_data_data)
+            .transaction_info(mock_transaction_info())
+            .build();
+
+    let rtx = ResolvedTransaction {
+        transaction,
+        resolved_cell_deps: vec![exec_caller_cell, exec_callee_cell, exec_caller_data_cell],
+        resolved_inputs: vec![dummy_cell],
+        resolved_dep_groups: vec![],
+    };
+
+    let proivder = MockDataLoader {};
+    let hardfork_switch = HardForkSwitch::new_without_any_enabled()
+        .as_builder()
+        .rfc_0032(0)
+        .build()
+        .unwrap();
+    let consensus = ConsensusBuilder::default()
+        .hardfork_switch(hardfork_switch)
+        .build();
+    let tx_verify_env =
+        TxVerifyEnv::new_submit(&HeaderView::new_advanced_builder().epoch(0.pack()).build());
+    let verifier = TransactionScriptsVerifier::new(&rtx, &consensus, &proivder, &tx_verify_env);
+
+    let result = verifier.verify(10_000_000_000);
+    assert!(result.is_ok());
+}
+
+fuzz_target!(|data: FuzzData| {
+    run(data);
+});

script/fuzz/programs/Makefile

@@ -0,0 +1,10 @@
+BUILDER_DOCKER=nervos/ckb-riscv-gnu-toolchain:bionic-20210804
+
+all:
+	riscv64-unknown-elf-gcc -o exec_callee exec_callee.c
+	riscv64-unknown-elf-gcc -o exec_caller exec_caller.c
+
+all-via-docker:
+	docker run --rm -v `pwd`:/code ${BUILDER_DOCKER} bash -c "cd /code && make all"
+
+.PHONY: all all-via-docker

script/fuzz/programs/exec_callee.c

@@ -0,0 +1,12 @@
+#include <string.h>
+
+int main(int argc, char* argv[]) {
+    int s = 0;
+    for (int i = 0; i < argc; i++) {
+        s += strlen(argv[i]);
+    }
+    if (s % 256 != 0) {
+        return 0;
+    }
+    return s;
+}

script/fuzz/programs/exec_caller.c

@@ -0,0 +1,75 @@
+#include <stdint.h>
+
+static inline long __internal_syscall(long n, long _a0, long _a1, long _a2,
+                                      long _a3, long _a4, long _a5) {
+  register long a0 asm("a0") = _a0;
+  register long a1 asm("a1") = _a1;
+  register long a2 asm("a2") = _a2;
+  register long a3 asm("a3") = _a3;
+  register long a4 asm("a4") = _a4;
+  register long a5 asm("a5") = _a5;
+
+#ifdef __riscv_32e
+  register long syscall_id asm("t0") = n;
+#else
+  register long syscall_id asm("a7") = n;
+#endif
+
+  asm volatile("scall"
+               : "+r"(a0)
+               : "r"(a1), "r"(a2), "r"(a3), "r"(a4), "r"(a5), "r"(syscall_id));
+  return a0;
+}
+
+#define syscall(n, a, b, c, d, e, f)                                           \
+  __internal_syscall(n, (long)(a), (long)(b), (long)(c), (long)(d), (long)(e), \
+                     (long)(f))
+
+uint64_t get_u64(uint8_t *buf) {
+  return ((uint64_t)buf[0] << 0x00) + ((uint64_t)buf[1] << 0x08) +
+         ((uint64_t)buf[2] << 0x10) + ((uint64_t)buf[3] << 0x18) +
+         ((uint64_t)buf[4] << 0x20) + ((uint64_t)buf[5] << 0x28) +
+         ((uint64_t)buf[6] << 0x30) + ((uint64_t)buf[7] << 0x38);
+}
+
+int main() {
+  uint8_t buf[262144] = {};
+  uint64_t len = 262144;
+  if (syscall(2092, buf, &len, 0, 2, 3, 0) != 0) {
+    return 1;
+  }
+
+  uint64_t p = 0;
+  uint8_t callee_from = buf[p];
+  p += 1;
+
+  uint64_t callee_offset = buf[p];
+  p += 1;
+
+  uint64_t callee_length = get_u64(&buf[p]);
+  p += 8;
+
+  uint64_t argc = get_u64(&buf[p]);
+  p += 8;
+
+  char *argv[262144] = {};
+  for (int i = 0; i < argc; i++) {
+    uint64_t l = get_u64(&buf[p]);
+    p += 8;
+    argv[i] = &buf[p];
+  }
+
+  if (callee_from == 0) {
+    // Callee from dep cell
+    syscall(2043, 1, 3, 0, (callee_offset << 32) | callee_length, argc, argv);
+  } else if (callee_from == 1) {
+    // Callee from witness input
+    syscall(2043, 0, 1, 1, (callee_offset << 32) | callee_length, argc, argv);
+  } else if (callee_from == 2) {
+    // Callee from witness output
+    syscall(2043, 0, 2, 1, (callee_offset << 32) | callee_length, argc, argv);
+  } else {
+    return 1;
+  }
+  return 1;
+}

script/src/syscalls/exec.rs

@@ -1,5 +1,7 @@
 use crate::cost_model::transferred_byte_cycles;
-use crate::syscalls::{Source, SourceEntry, EXEC, INDEX_OUT_OF_BOUND, WRONG_FORMAT};
+use crate::syscalls::{
+    Source, SourceEntry, EXEC, INDEX_OUT_OF_BOUND, SLICE_OUT_OF_BOUND, WRONG_FORMAT,
+};
 use ckb_traits::CellDataProvider;
 use ckb_types::core::cell::CellMeta;
 use ckb_types::packed::{Bytes as PackedBytes, BytesVec};
@@ -130,7 +132,7 @@ impl<'a, Mac: SupportMachine, DL: CellDataProvider> Syscalls<Mac> for Exec<'a, D
             let cell = self.fetch_cell(source, index as usize);
             if let Err(err) = cell {
                 machine.set_register(A0, Mac::REG::from_u8(err));
-                return Ok(false);
+                return Ok(true);
             }
             let cell = cell.unwrap();
             self.data_loader
@@ -145,10 +147,20 @@ impl<'a, Mac: SupportMachine, DL: CellDataProvider> Syscalls<Mac> for Exec<'a, D
             let witness = witness.unwrap();
             witness.raw_data()
         };
+        let data_size = data.len();
+        if offset >= data_size {
+            machine.set_register(A0, Mac::REG::from_u8(SLICE_OUT_OF_BOUND));
+            return Ok(true);
+        };
         let data = if length == 0 {
-            data.slice(offset..data.len())
+            data.slice(offset..data_size)
         } else {
-            data.slice(offset..offset + length)
+            let end = offset.checked_add(length).ok_or(VMError::OutOfBound)?;
+            if end >= data_size {
+                machine.set_register(A0, Mac::REG::from_u8(SLICE_OUT_OF_BOUND));
+                return Ok(true);
+            }
+            data.slice(offset..end)
         };
         let argc = machine.registers()[A4].to_u64();
         let mut addr = machine.registers()[A5].to_u64();
@@ -175,7 +187,7 @@ impl<'a, Mac: SupportMachine, DL: CellDataProvider> Syscalls<Mac> for Exec<'a, D
             }
             Err(_) => {
                 machine.set_register(A0, Mac::REG::from_u8(WRONG_FORMAT));
-                return Ok(false);
+                return Ok(true);
             }
         }
 
@@ -189,7 +201,7 @@ impl<'a, Mac: SupportMachine, DL: CellDataProvider> Syscalls<Mac> for Exec<'a, D
             }
             Err(_) => {
                 machine.set_register(A0, Mac::REG::from_u8(WRONG_FORMAT));
-                return Ok(false);
+                return Ok(true);
             }
         }
         Ok(true)