security fixing for multiple codes

crates/rooch-framework/src/natives/rooch_framework/crypto/decoding.rs

@@ -108,9 +108,15 @@ pub fn native_bech32(
         gas_params.base + (gas_params.per_byte * NumBytes::new(data.as_bytes_ref().len() as u64));
 
     let binding = data.as_bytes_ref();
-    let data_str = std::str::from_utf8(&binding).unwrap();
+    let data_str = match std::str::from_utf8(&binding) {
+        Ok(v) => v,
+        Err(_) => return Ok(NativeResult::err(cost, E_DECODE_FAILED)),
+    };
 
-    let (hrp, public_key, variant) = bech32::decode(data_str).unwrap();
+    let (hrp, public_key, variant) = match bech32::decode(data_str) {
+        Ok((v1, v2, v3)) => (v1, v2, v3),
+        Err(_) => return Ok(NativeResult::err(cost, E_DECODE_FAILED)),
+    };
 
     if hrp != "bech32" && hrp != "bech32m" {
         return Ok(NativeResult::err(cost, E_DECODE_FAILED));

crates/rooch-framework/src/natives/rooch_framework/crypto/ecdsa_k1.rs

@@ -43,9 +43,11 @@ pub fn native_ecrecover(
     let signature = pop_arg!(args, VectorRef);
     let msg_ref = msg.as_bytes_ref();
     let signature_ref = signature.as_bytes_ref();
+    let msg_size = msg_ref.len();
+    let signature_len = signature_ref.len();
 
-    // TODO(Gas): Charge the arg size dependent costs
-    let cost = gas_params.base;
+    let cost =
+        gas_params.base + gas_params.per_byte * NumBytes::new((msg_size + signature_len) as u64);
 
     let Ok(sig) = <Secp256k1RecoverableSignature as ToFromBytes>::from_bytes(&signature_ref) else {
         return Ok(NativeResult::err(cost, E_INVALID_SIGNATURE));
@@ -78,8 +80,8 @@ pub fn native_decompress_pubkey(
     let pubkey = pop_arg!(args, VectorRef);
     let pubkey_ref = pubkey.as_bytes_ref();
 
-    // TODO(Gas): Charge the arg size dependent costs
-    let cost = gas_params.base;
+    let pubkey_ref_len = pubkey_ref.len();
+    let cost = gas_params.base + gas_params.per_byte * NumBytes::new(pubkey_ref_len as u64);
 
     match Secp256k1PublicKey::from_bytes(&pubkey_ref) {
         Ok(pubkey) => {

crates/rooch-rpc-server/src/server/btc_server.rs

@@ -66,9 +66,14 @@ impl BtcAPIServer for BtcServer {
 
         let global_state_filter =
             UTXOFilterView::into_global_state_filter(filter, resolve_address)?;
+        let limit_value = limit_of
+            .checked_add(1)
+            .ok_or(jsonrpsee::core::Error::Custom(
+                "limit value overflow".to_string(),
+            ))?;
         let states = self
             .rpc_service
-            .query_object_states(global_state_filter, cursor, limit_of + 1, descending_order)
+            .query_object_states(global_state_filter, cursor, limit_value, descending_order)
             .await?;
 
         let mut data = self
@@ -121,9 +126,14 @@ impl BtcAPIServer for BtcServer {
 
         let global_state_filter =
             InscriptionFilterView::into_global_state_filter(filter, resolve_address)?;
+        let limit_value = limit_of
+            .checked_add(1)
+            .ok_or(jsonrpsee::core::Error::Custom(
+                "limit value overflow".to_string(),
+            ))?;
         let states = self
             .rpc_service
-            .query_object_states(global_state_filter, cursor, limit_of + 1, descending_order)
+            .query_object_states(global_state_filter, cursor, limit_value, descending_order)
             .await?;
 
         let mut data = self

crates/rooch-rpc-server/src/server/rooch_server.rs

@@ -231,11 +231,16 @@ impl RoochAPIServer for RoochServer {
             Some(key_state_str) => Some(KeyState::from_str(key_state_str.as_str())?),
             None => None,
         };
+        let limit_value = limit_of
+            .checked_add(1)
+            .ok_or(jsonrpsee::core::Error::Custom(
+                "limit value overflow".to_string(),
+            ))?;
         let mut data: Vec<StateKVView> = if state_option.decode || show_display {
             let is_object = access_path.0.is_object();
             let (key_states, states): (Vec<AnnotatedKeyState>, Vec<AnnotatedState>) = self
                 .rpc_service
-                .list_annotated_states(access_path.into(), cursor_of, limit_of + 1)
+                .list_annotated_states(access_path.into(), cursor_of, limit_value)
                 .await?
                 .into_iter()
                 .unzip();
@@ -266,7 +271,7 @@ impl RoochAPIServer for RoochServer {
             }
         } else {
             self.rpc_service
-                .list_states(access_path.into(), cursor_of, limit_of + 1)
+                .list_states(access_path.into(), cursor_of, limit_value)
                 .await?
                 .into_iter()
                 .map(|(key_state, state)| {
@@ -303,7 +308,12 @@ impl RoochAPIServer for RoochServer {
 
         // NOTE: fetch one more object to check if there is next page
         let limit_of = min(limit.unwrap_or(DEFAULT_RESULT_LIMIT), MAX_RESULT_LIMIT);
-        let limit = limit_of + 1;
+        let limit_value = limit_of
+            .checked_add(1)
+            .ok_or(jsonrpsee::core::Error::Custom(
+                "limit value overflow".to_string(),
+            ))?;
+        let limit = limit_value;
         let mut data = if event_options.decode {
             self.rpc_service
                 .get_annotated_events_by_event_handle(
@@ -379,15 +389,27 @@ impl RoochAPIServer for RoochServer {
 
         let tx_orders = if descending_order {
             let start = cursor.unwrap_or(last_sequencer_order + 1);
-            let end = if start >= limit_of {
-                start - limit_of
-            } else {
-                0
-            };
+            let start_sub = start
+                .checked_sub(limit_of)
+                .ok_or(jsonrpsee::core::Error::Custom(
+                    "cursor value overflow".to_string(),
+                ))?;
+            let end = if start >= limit_of { start_sub } else { 0 };
             (end..start).rev().collect::<Vec<_>>()
         } else {
             let start = cursor.unwrap_or(0);
-            let end = min(start + (limit_of + 1), last_sequencer_order + 1);
+            let limit_value = limit_of
+                .checked_add(1)
+                .ok_or(jsonrpsee::core::Error::Custom(
+                    "limit value overflow".to_string(),
+                ))?;
+            let start_plus =
+                start
+                    .checked_add(limit_value)
+                    .ok_or(jsonrpsee::core::Error::Custom(
+                        "cursor value overflow".to_string(),
+                    ))?;
+            let end = min(start_plus, last_sequencer_order + 1);
             (start..end).collect::<Vec<_>>()
         };
 
@@ -453,9 +475,14 @@ impl RoochAPIServer for RoochServer {
             None => None,
         };
 
+        let limit_value = limit_of
+            .checked_add(1)
+            .ok_or(jsonrpsee::core::Error::Custom(
+                "limit value overflow".to_string(),
+            ))?;
         let mut data = self
             .aggregate_service
-            .get_balances(account_addr.into(), cursor_of, limit_of + 1)
+            .get_balances(account_addr.into(), cursor_of, limit_value)
             .await?;
 
         let has_next_page = data.len() > limit_of;
@@ -491,9 +518,14 @@ impl RoochAPIServer for RoochServer {
         let cursor = cursor.map(|v| v.0);
         let descending_order = descending_order.unwrap_or(true);
 
+        let limit_value = limit_of
+            .checked_add(1)
+            .ok_or(jsonrpsee::core::Error::Custom(
+                "limit value overflow".to_string(),
+            ))?;
         let mut data = self
             .rpc_service
-            .query_transactions(filter.into(), cursor, limit_of + 1, descending_order)
+            .query_transactions(filter.into(), cursor, limit_value, descending_order)
             .await?;
 
         let has_next_page = data.len() > limit_of;
@@ -527,9 +559,14 @@ impl RoochAPIServer for RoochServer {
         );
         let descending_order = descending_order.unwrap_or(true);
 
+        let limit_value = limit_of
+            .checked_add(1)
+            .ok_or(jsonrpsee::core::Error::Custom(
+                "limit value overflow".to_string(),
+            ))?;
         let mut data = self
             .rpc_service
-            .query_events(filter.into(), cursor, limit_of + 1, descending_order)
+            .query_events(filter.into(), cursor, limit_value, descending_order)
             .await?
             .into_iter()
             .map(IndexerEventView::from)
@@ -581,9 +618,14 @@ impl RoochAPIServer for RoochServer {
         };
         let global_state_filter =
             ObjectStateFilterView::into_object_state_filter(filter, resolve_address);
+        let limit_value = limit_of
+            .checked_add(1)
+            .ok_or(jsonrpsee::core::Error::Custom(
+                "limit value overflow".to_string(),
+            ))?;
         let mut data = self
             .rpc_service
-            .query_object_states(global_state_filter, cursor, limit_of + 1, descending_order)
+            .query_object_states(global_state_filter, cursor, limit_value, descending_order)
             .await?
             .into_iter()
             .map(IndexerObjectStateView::try_new_from_global_state)
@@ -616,9 +658,14 @@ impl RoochAPIServer for RoochServer {
         );
         let descending_order = descending_order.unwrap_or(true);
 
+        let limit_value = limit_of
+            .checked_add(1)
+            .ok_or(jsonrpsee::core::Error::Custom(
+                "limit value overflow".to_string(),
+            ))?;
         let mut data = self
             .rpc_service
-            .query_field_states(filter.into(), cursor, limit_of + 1, descending_order)
+            .query_field_states(filter.into(), cursor, limit_value, descending_order)
             .await?
             .into_iter()
             .map(IndexerFieldStateView::try_new_from_table_state)

moveos/moveos-stdlib/src/natives/moveos_stdlib/wasm.rs

@@ -32,6 +32,11 @@ const E_WASM_FUNCTION_NOT_FOUND: u64 = 6;
 const E_WASM_MEMORY_ACCESS_FAILED: u64 = 7;
 const E_JSON_UNMARSHAL_FAILED: u64 = 8;
 pub const E_CBOR_MARSHAL_FAILED: u64 = 9;
+pub const E_EMPTY_RETURN_VALUE: u64 = 10;
+pub const E_VALUE_NOT_I32: u64 = 11;
+pub const E_MEMORY_NOT_FOUND: u64 = 12;
+pub const E_INCORRECT_LENGTH_OF_ARGS: u64 = 13;
+pub const E_CBOR_UNMARSHAL_FAILED: u64 = 14;
 
 #[derive(Debug, Clone)]
 pub struct WASMCreateInstanceGasParameters {
@@ -131,8 +136,10 @@ fn native_create_cbor_values(
         }
     }
 
-    let cbor_value: ciborium::Value =
-        ciborium::from_reader(cbor_buffer.as_slice()).expect("cbor unmarshal failed");
+    let cbor_value: ciborium::Value = match ciborium::from_reader(cbor_buffer.as_slice()) {
+        Ok(v) => v,
+        Err(_) => return build_err(gas_params.base, E_CBOR_UNMARSHAL_FAILED),
+    };
     log::debug!(
         "native_create_cbor_values -> mint_args_array {:?}",
         cbor_value
@@ -242,11 +249,10 @@ fn native_create_wasm_args_in_memory(
             ));
         }
         Some(instance) => {
-            let stack_alloc_func = instance
-                .instance
-                .exports
-                .get_function("stackAlloc")
-                .unwrap();
+            let stack_alloc_func = match instance.instance.exports.get_function("stackAlloc") {
+                Ok(v) => v,
+                Err(_) => return build_err(gas_params.base_create_args, E_WASM_FUNCTION_NOT_FOUND),
+            };
 
             for arg in func_args.iter() {
                 let c_arg = unsafe { CString::from_vec_unchecked(arg.clone()) };
@@ -288,6 +294,10 @@ impl WASMExecuteGasParameters {
     }
 }
 
+fn build_err(cost: InternalGas, abort_code: u64) -> PartialVMResult<NativeResult> {
+    return Ok(NativeResult::err(cost, abort_code));
+}
+
 // native_execute_wasm_function
 #[inline]
 fn native_execute_wasm_function(
@@ -322,8 +332,24 @@ fn native_execute_wasm_function(
 
                     match calling_function.call(&mut instance.store, wasm_func_args.as_slice()) {
                         Ok(ret) => {
-                            let return_value = ret.deref().get(0).unwrap();
-                            let offset = return_value.i32().unwrap();
+                            let return_value = match ret.deref().get(0) {
+                                Some(v) => v,
+                                None => {
+                                    return build_err(
+                                        gas_params.base_create_execution,
+                                        E_EMPTY_RETURN_VALUE,
+                                    )
+                                }
+                            };
+                            let offset = match return_value.i32() {
+                                Some(v) => v,
+                                None => {
+                                    return build_err(
+                                        gas_params.base_create_execution,
+                                        E_VALUE_NOT_I32,
+                                    )
+                                }
+                            };
                             let ret_val = Value::u64(offset as u64);
 
                             let mut cost = gas_params.base_create_execution;
@@ -381,7 +407,10 @@ fn native_read_data_length(
     let ret = match instance_pool.lock().unwrap().get_mut(&instance_id) {
         None => Ok(NativeResult::err(gas_params.base, E_INSTANCE_NO_EXISTS)),
         Some(instance) => {
-            let memory = instance.instance.exports.get_memory("memory").unwrap();
+            let memory = match instance.instance.exports.get_memory("memory") {
+                Ok(v) => v,
+                Err(_) => return build_err(gas_params.base, E_MEMORY_NOT_FOUND),
+            };
             let memory_view = memory.view(&instance.store);
             let mut length_bytes: [u8; 4] = [0; 4];
             match memory_view.read(data_ptr, length_bytes.as_mut_slice()) {
@@ -435,7 +464,10 @@ fn native_read_data_from_heap(
     let ret = match instance_pool.lock().unwrap().get_mut(&instance_id) {
         None => Ok(NativeResult::err(gas_params.base, E_INSTANCE_NO_EXISTS)),
         Some(instance) => {
-            let memory = instance.instance.exports.get_memory("memory").unwrap();
+            let memory = match instance.instance.exports.get_memory("memory") {
+                Ok(v) => v,
+                Err(_) => return build_err(gas_params.base, E_MEMORY_NOT_FOUND),
+            };
             let memory_view = memory.view(&instance.store);
             if data_length > 0 {
                 let mut data = vec![0; data_length as usize];
@@ -506,7 +538,10 @@ fn native_release_wasm_instance(
     _ty_args: Vec<Type>,
     mut args: VecDeque<Value>,
 ) -> PartialVMResult<NativeResult> {
-    let value = args.pop_back().unwrap();
+    let value = match args.pop_back() {
+        Some(v) => v,
+        None => return build_err(gas_params.base, E_INCORRECT_LENGTH_OF_ARGS),
+    };
     let mut fiedls = value.value_as::<Struct>()?.unpack()?;
     let val = fiedls.next().ok_or_else(|| {
         PartialVMError::new(StatusCode::TYPE_RESOLUTION_FAILURE)

moveos/moveos-types/src/moveos_std/tx_meta.rs

@@ -44,17 +44,27 @@ impl TxMeta {
 
     pub fn new_from_move_action(move_action: &MoveAction) -> Self {
         let function_meta = match move_action {
-            MoveAction::Function(call) => Some(FunctionCallMeta {
-                module_address: *call.function_id.module_id.address(),
-                module_name: MoveAsciiString::new(
-                    call.function_id.module_id.name().as_bytes().to_vec(),
-                )
-                .expect("module name must be valid ascii"),
-                function_name: MoveAsciiString::new(
-                    call.function_id.function_name.as_bytes().to_vec(),
-                )
-                .expect("module name must be valid ascii"),
-            }),
+            MoveAction::Function(call) => {
+                let module_name =
+                    MoveAsciiString::new(call.function_id.module_id.name().as_bytes().to_vec());
+                let function_name =
+                    MoveAsciiString::new(call.function_id.function_name.as_bytes().to_vec());
+                if module_name.is_err() || function_name.is_err() {
+                    None
+                } else {
+                    Some(FunctionCallMeta {
+                        module_address: *call.function_id.module_id.address(),
+                        module_name: MoveAsciiString::new(
+                            call.function_id.module_id.name().as_bytes().to_vec(),
+                        )
+                        .expect("module name must be valid ascii"),
+                        function_name: MoveAsciiString::new(
+                            call.function_id.function_name.as_bytes().to_vec(),
+                        )
+                        .expect("module name must be valid ascii"),
+                    })
+                }
+            }
             _ => None,
         };
         Self {

moveos/moveos-verifier/src/metadata.rs

@@ -195,7 +195,6 @@ impl<'a> ExtendedChecker<'a> {
         // Inspect the bytecode of every function, and if an instruction is CallGeneric,
         // verify that it calls a function with the private_generics attribute as detected earlier.
         // Then, ensure that the generic parameters of the CallGeneric instruction are valid.
-        // #TODO: Ensure that every generic function call is fully checked.
         check_call_generics(self.env, module, view);
 
         for (private_generics_func_name, types_list) in type_name_indices {