Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor!: call reserved system contract address is forbidden #1597

Merged
merged 3 commits into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions core/api/src/jsonrpc/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ pub enum RpcError {
InvalidFromBlockAndToBlockUnion,
#[display(fmt = "Invalid filter id {}", _0)]
CannotFindFilterId(u64),
#[display(fmt = "Not allow to call system contract address")]
CallSystemContract,

#[display(fmt = "EVM error {}", "decode_revert_msg(&_0.ret)")]
Evm(TxResp),
Expand Down Expand Up @@ -88,6 +90,7 @@ impl RpcError {
RpcError::InvalidRewardPercentiles(_, _) => -40020,
RpcError::InvalidFromBlockAndToBlockUnion => -40021,
RpcError::CannotFindFilterId(_) => -40022,
RpcError::CallSystemContract => -40023,

RpcError::Evm(_) => -49998,
RpcError::Internal(_) => -49999,
Expand Down Expand Up @@ -129,6 +132,7 @@ impl From<RpcError> for ErrorObjectOwned {
ErrorObject::owned(err_code, err, none_data)
}
RpcError::CannotFindFilterId(_) => ErrorObject::owned(err_code, err, none_data),
RpcError::CallSystemContract => ErrorObject::owned(err_code, err, none_data),

RpcError::Evm(resp) => {
ErrorObject::owned(err_code, err.clone(), Some(vm_err(resp.clone())))
Expand Down
17 changes: 17 additions & 0 deletions core/api/src/jsonrpc/impl/web3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::{sync::Arc, time::Duration};
use jsonrpsee::core::RpcResult;

use common_apm::metrics_rpc;
use core_executor::is_system_contract_address_format;
use protocol::traits::{APIAdapter, Context};
use protocol::types::{
Block, BlockNumber, Bytes, EthAccountProof, Hash, Header, Hex, Proposal, Receipt,
Expand Down Expand Up @@ -419,6 +420,12 @@ impl<Adapter: APIAdapter + 'static> Web3RpcServer for Web3RpcImpl<Adapter> {
return Err(RpcError::GasLimitIsTooLarge.into());
}

if let Some(call_addr) = req.to {
if is_system_contract_address_format(&call_addr) {
return Err(RpcError::CallSystemContract.into());
}
}

let number = self.get_block_number_by_id(block_id).await?;

let data_bytes = req
Expand Down Expand Up @@ -453,6 +460,12 @@ impl<Adapter: APIAdapter + 'static> Web3RpcServer for Web3RpcImpl<Adapter> {
}
}

if let Some(call_addr) = req.to {
if is_system_contract_address_format(&call_addr) {
return Err(RpcError::CallSystemContract.into());
}
}

let num = match number {
Some(BlockId::Num(n)) => Some(n.as_u64()),
_ => None,
Expand Down Expand Up @@ -1011,6 +1024,10 @@ impl<Adapter: APIAdapter + 'static> Web3RpcServer for Web3RpcImpl<Adapter> {
storage_position: Vec<U256>,
number: BlockId,
) -> RpcResult<EthAccountProof> {
if is_system_contract_address_format(&address) {
return Err(RpcError::CallSystemContract.into());
}

let number = self.get_block_number_by_id(Some(number)).await?;

let header = self
Expand Down
19 changes: 3 additions & 16 deletions core/executor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub use crate::adapter::{
AxonExecutorApplyAdapter, AxonExecutorReadOnlyAdapter, MPTTrie, RocksTrieDB,
};
pub use crate::system_contract::{
is_call_system_script, is_system_contract_address_format,
metadata::{MetadataHandle, HARDFORK_INFO},
DataProvider,
};
Expand All @@ -34,8 +35,8 @@ use protocol::types::{
use crate::precompiles::build_precompile_set;
use crate::system_contract::{
after_block_hook, before_block_hook, system_contract_dispatch,
CKB_LIGHT_CLIENT_CONTRACT_ADDRESS, HEADER_CELL_ROOT_KEY, IMAGE_CELL_CONTRACT_ADDRESS,
METADATA_CONTRACT_ADDRESS, METADATA_ROOT_KEY, NATIVE_TOKEN_CONTRACT_ADDRESS,
CKB_LIGHT_CLIENT_CONTRACT_ADDRESS, HEADER_CELL_ROOT_KEY, METADATA_CONTRACT_ADDRESS,
METADATA_ROOT_KEY,
};

lazy_static::lazy_static! {
Expand Down Expand Up @@ -491,20 +492,6 @@ impl AxonExecutor {
}
}

pub fn is_call_system_script(action: &TransactionAction) -> bool {
let system_contracts = [
NATIVE_TOKEN_CONTRACT_ADDRESS,
METADATA_CONTRACT_ADDRESS,
CKB_LIGHT_CLIENT_CONTRACT_ADDRESS,
IMAGE_CELL_CONTRACT_ADDRESS,
];

match action {
TransactionAction::Call(addr) => system_contracts.contains(addr),
TransactionAction::Create => false,
}
}

pub fn is_transaction_call(action: &TransactionAction, addr: &H160) -> bool {
action == &TransactionAction::Call(*addr)
}
Expand Down
5 changes: 4 additions & 1 deletion core/executor/src/system_contract/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::io;
use ethers::abi::AbiError;
use thiserror::Error;

use protocol::{ProtocolError, ProtocolErrorKind};
use protocol::{types::H160, ProtocolError, ProtocolErrorKind};

#[derive(Error, Debug)]
pub enum SystemScriptError {
Expand Down Expand Up @@ -84,6 +84,9 @@ pub enum SystemScriptError {

#[error("Metadata version is discontinuous")]
MetadataVersionDiscontinuity,

#[error("Call a reserved system contract address {0}")]
ReservedAddress(H160),
}

impl From<SystemScriptError> for ProtocolError {
Expand Down
75 changes: 74 additions & 1 deletion core/executor/src/system_contract/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub mod metadata;
pub use crate::system_contract::ckb_light_client::{
CkbLightClientContract, CKB_LIGHT_CLIENT_CONTRACT_ADDRESS,
};
use crate::system_contract::error::SystemScriptError;
pub use crate::system_contract::image_cell::{ImageCellContract, IMAGE_CELL_CONTRACT_ADDRESS};
pub use crate::system_contract::metadata::{
check_ckb_related_info_exist, MetadataContract, METADATA_CONTRACT_ADDRESS,
Expand All @@ -29,7 +30,8 @@ use rocksdb::DB;

use protocol::traits::{CkbDataProvider, ExecutorAdapter};
use protocol::types::{
Bytes, HardforkInfoInner, Hasher, Metadata, SignedTransaction, TxResp, H160, H256,
Bytes, HardforkInfoInner, Hasher, Metadata, SignedTransaction, TransactionAction, TxResp, H160,
H256,
};
use protocol::{ckb_blake2b_256, ProtocolResult};

Expand All @@ -45,6 +47,17 @@ pub const fn system_contract_address(addr: u8) -> H160 {
0xff, 0xff, 0xff, 0xff, addr,
])
}
const SYSTEM_CONTRACT_ADDRESSES_PREFIX: [u8; 19] = [
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, // 0xff * 8
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, // 0xff * 8
0xff, 0xff, 0xff, // 0xff * 3
];
const SYSTEM_CONTRACT_ADDRESSES_SET: [H160; 4] = [
NATIVE_TOKEN_CONTRACT_ADDRESS,
METADATA_CONTRACT_ADDRESS,
CKB_LIGHT_CLIENT_CONTRACT_ADDRESS,
IMAGE_CELL_CONTRACT_ADDRESS,
];
const HEADER_CELL_DB_CACHE_SIZE: usize = 200;
const METADATA_DB_CACHE_SIZE: usize = 10;

Expand Down Expand Up @@ -304,3 +317,63 @@ impl DataProvider {
DataProvider { root }
}
}

pub fn is_system_contract_address_format(addr: &H160) -> bool {
addr.0[0..19] == SYSTEM_CONTRACT_ADDRESSES_PREFIX
}

pub fn is_call_system_script(action: &TransactionAction) -> ProtocolResult<bool> {
let call_addr = match action {
TransactionAction::Call(addr) => addr,
TransactionAction::Create => return Ok(false),
};

// The first 19 bytes of the address are 0xff, which means that the address
// follows system contract address format.
if call_addr.0[0..19] == SYSTEM_CONTRACT_ADDRESSES_PREFIX {
if SYSTEM_CONTRACT_ADDRESSES_SET.contains(call_addr) {
return Ok(true);
}

// Call a reserved system contract address returns error.
return Err(SystemScriptError::ReservedAddress(*call_addr).into());
}

// The address is not a system contract address.
Ok(false)
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_is_call_system_contract() {
let action = TransactionAction::Create;
assert!(!is_call_system_script(&action).unwrap());

let addr = H160::from_low_u64_be(0x1);
let action = TransactionAction::Call(addr);
assert!(!is_call_system_script(&action).unwrap());

let addr = NATIVE_TOKEN_CONTRACT_ADDRESS;
let action = TransactionAction::Call(addr);
assert!(is_call_system_script(&action).unwrap());

let addr = METADATA_CONTRACT_ADDRESS;
let action = TransactionAction::Call(addr);
assert!(is_call_system_script(&action).unwrap());

let addr = CKB_LIGHT_CLIENT_CONTRACT_ADDRESS;
let action = TransactionAction::Call(addr);
assert!(is_call_system_script(&action).unwrap());

let addr = IMAGE_CELL_CONTRACT_ADDRESS;
let action = TransactionAction::Call(addr);
assert!(is_call_system_script(&action).unwrap());

let addr = system_contract_address(0x4);
let action = TransactionAction::Call(addr);
assert!(is_call_system_script(&action).is_err());
}
}
2 changes: 1 addition & 1 deletion core/mempool/src/adapter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ where
ctx: Context,
tx: &SignedTransaction,
) -> ProtocolResult<U256> {
if is_call_system_script(tx.transaction.unsigned.action()) {
if is_call_system_script(tx.transaction.unsigned.action())? {
return self.check_system_script_tx_authorization(ctx, tx).await;
}

Expand Down
4 changes: 2 additions & 2 deletions core/mempool/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ where
Adapter: MemPoolAdapter + 'static,
{
async fn insert(&self, ctx: Context, tx: SignedTransaction) -> ProtocolResult<()> {
let is_call_system_script = is_call_system_script(tx.transaction.unsigned.action());
let is_call_system_script = is_call_system_script(tx.transaction.unsigned.action())?;

log::debug!(
"[mempool]: is call system script {:?}",
Expand Down Expand Up @@ -306,7 +306,7 @@ where

for (signed_tx, check_nonce) in txs.into_iter().zip(check_nonces.into_iter()) {
let is_call_system_script =
is_call_system_script(signed_tx.transaction.unsigned.action());
is_call_system_script(signed_tx.transaction.unsigned.action())?;
if is_call_system_script {
self.pool.insert_system_script_tx(signed_tx)?;
} else {
Expand Down