Skip to content

Commit

Permalink
Prevent coin transfer to SC addresses
Browse files Browse the repository at this point in the history
  • Loading branch information
@damip
damip committed Jun 2, 2023
1 parent 07a9f9d commit 68db6bd
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 45 deletions.
80 changes: 59 additions & 21 deletions massa-execution-worker/src/context.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use massa_ledger_exports::LedgerChanges;
use massa_models::address::ExecutionAddressCycleInfo;
use massa_models::bytecode::Bytecode;
use massa_models::denunciation::DenunciationIndex;
use massa_models::timeslots::get_block_slot_timestamp;
use massa_models::{
address::Address,
amount::Amount,
Expand All @@ -36,7 +37,7 @@ use massa_module_cache::controller::ModuleCache;
use massa_pos_exports::PoSChanges;
use massa_versioning::address_factory::{AddressArgs, AddressFactory};
use massa_versioning::versioning::MipStore;
use massa_versioning::versioning_factory::VersioningFactory;
use massa_versioning::versioning_factory::{FactoryStrategy, VersioningFactory};
use parking_lot::RwLock;
use rand::SeedableRng;
use rand_xoshiro::Xoshiro256PlusPlus;
Expand Down Expand Up @@ -454,32 +455,57 @@ impl ExecutionContext {

/// Creates a new smart contract address with initial bytecode, and returns this address
pub fn create_new_sc_address(&mut self, bytecode: Bytecode) -> Result<Address, ExecutionError> {
// TODO: collision problem:
// prefix addresses to know if they are SCs or normal,
// otherwise people can already create new accounts by sending coins to the right hash
// they won't have ownership over it but this can still be unexpected
// to have initial extra coins when an address is created
// It may also induce that for read-only calls.
// https://github.com/massalabs/massa/issues/2331

// deterministically generate a new unique smart contract address

let slot_timestamp = get_block_slot_timestamp(
self.config.thread_count,
self.config.t0,
self.config.genesis_timestamp,
self.slot,
)
.expect("could not compute current slot timestamp");

// create a seed from the current slot
let mut data: Vec<u8> = self.slot.to_bytes_key().to_vec();
// add the index of the created address within this context to the seed
data.append(&mut self.created_addr_index.to_be_bytes().to_vec());
data.extend(self.created_addr_index.to_be_bytes());
// add a flag on whether we are in read-only mode or not to the seed
// this prevents read-only contexts from shadowing existing addresses
if self.read_only {
data.push(0u8);
} else {
data.push(1u8);
}
// hash the seed to get a unique address
let hash = Hash::compute_from(&data);
let address = self
.address_factory
.create(&AddressArgs::SC { hash }, None)?;
// hash the seed to get an address
let mut hash = Hash::compute_from(&data);
let mut nonce = 0u64;
// Loop over nonces until we find an address that doesn't exist in the speculative ledger.
// Note that this loop is here for robustness, and should not be looping because
// even through the SC addresses are predictable, nobody can create them beforehand because:
// - their category is "SC" and not "USER" so they can't be derived from a public key
// - sending tokens to the target SC address to create it by funding is not allowed because transactions towards SC addresses are not allowed
loop {
let address = self.address_factory.create(
&AddressArgs::SC { hash },
Some(FactoryStrategy::At(slot_timestamp)),
)?;
// check if this address already exists in the speculative ledger
if !self.speculative_ledger.contains_address(&address) {
// if not, we can use it
break;
}
// otherwise, increment the nonce to get a new hash and try again
nonce = nonce.checked_add(1).ok_or_else(|| {
ExecutionError::RuntimeError("nonce overflow when creating SC address".into())
})?;
hash = Hash::compute_from(&[hash.as_bytes(), nonce.to_be_bytes()].concat());
}
if nonce > 0 {
warn!(
"smart contract address generation required {} nonces",
nonce
);
}

// add this address with its bytecode to the speculative ledger
self.speculative_ledger.create_new_sc_address(
Expand Down Expand Up @@ -655,13 +681,25 @@ impl ExecutionContext {
) -> Result<(), ExecutionError> {
if let Some(from_addr) = &from_addr {
// check access rights
if check_rights && !self.has_write_rights_on(from_addr) {
return Err(ExecutionError::RuntimeError(format!(
"spending from address {} is not allowed in this context",
from_addr
)));
}
if check_rights {
// ensure we can't spend from an address on which we have no write access
if !self.has_write_rights_on(from_addr) {
return Err(ExecutionError::RuntimeError(format!(
"spending from address {} is not allowed in this context",
from_addr
)));
}

// ensure we can't transfer towards SC addresses on which we have no write access
if let Some(to_addr) = &to_addr {
if to_addr.is_smart_contract() && !self.has_write_rights_on(to_addr) {
return Err(ExecutionError::RuntimeError(format!(
"crediting SC address {} is not allowed without write access to it",
to_addr
)));
}
}
}
// control vesting min_balance for sender address
self.vesting_manager
.check_vesting_transfer_coins(self, from_addr, amount)?;
Expand Down
47 changes: 26 additions & 21 deletions massa-execution-worker/src/execution.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,7 @@ impl ExecutionState {
)));
}

// add rolls to the buyer withing the context
// add rolls to the buyer within the context
context.add_rolls(&buyer_addr, *roll_count);

Ok(())
Expand Down Expand Up @@ -680,7 +680,7 @@ impl ExecutionState {

// send `roll_price` * `roll_count` coins from the sender to the recipient
if let Err(err) =
context.transfer_coins(Some(sender_addr), Some(*recipient_address), *amount, false)
context.transfer_coins(Some(sender_addr), Some(*recipient_address), *amount, true)
{
return Err(ExecutionError::TransactionError(format!(
"transfer of {} coins from {} to {} failed: {}",
Expand Down Expand Up @@ -804,19 +804,21 @@ impl ExecutionState {
},
];

// Debit the sender's balance with the coins to transfer
if let Err(err) = context.transfer_coins(Some(sender_addr), None, coins, false) {
// Ensure that the target address is an SC address
if !target_addr.is_smart_contract() {
return Err(ExecutionError::RuntimeError(format!(
"failed to debit operation sender {} with {} operation coins: {}",
sender_addr, coins, err
"cannot callSC towards non-SC address {}",
target_addr
)));
}

// Credit the operation target with coins.
if let Err(err) = context.transfer_coins(None, Some(target_addr), coins, false) {
// Transfer coins from the sender to the target
if let Err(err) =
context.transfer_coins(Some(sender_addr), Some(target_addr), coins, false)
{
return Err(ExecutionError::RuntimeError(format!(
"failed to credit operation target {} with {} operation coins: {}",
target_addr, coins, err
"failed to transfer {} operation coins from {} to {}: {}",
coins, sender_addr, target_addr, err
)));
}

Expand Down Expand Up @@ -890,18 +892,21 @@ impl ExecutionState {
},
];

// If there is no target bytecode or if message data is invalid,
// reimburse sender with coins and quit
// if the target address is not SC: fail
if !message.destination.is_smart_contract() {
let err = ExecutionError::RuntimeError(
"the target address is not a smart contract address".into(),
);
context.reset_to_snapshot(context_snapshot, err.clone());
context.cancel_async_message(&message);
return Err(err);
}

// if there is no bytecode: fail
let bytecode = match bytecode {
Some(bc) => bc,
bc => {
let err = if bc.is_none() {
ExecutionError::RuntimeError("no target bytecode found".into())
} else {
ExecutionError::RuntimeError(
"message data does not convert to utf-8".into(),
)
};
Some(bytecode) => bytecode,
None => {
let err = ExecutionError::RuntimeError("no target bytecode found".into());
context.reset_to_snapshot(context_snapshot, err.clone());
context.cancel_async_message(&message);
return Err(err);
Expand Down
20 changes: 17 additions & 3 deletions massa-execution-worker/src/interface_impl.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,11 @@ impl Interface for InterfaceImpl {
// get target address
let to_address = Address::from_str(address)?;

// check that the target address is an SC address
if !to_address.is_smart_contract() {
bail!("called address {} is not an SC address", to_address);
}

// write-lock context
let mut context = context_guard!(self);

Expand All @@ -187,7 +192,9 @@ impl Interface for InterfaceImpl {

// transfer coins from caller to target address
let coins = Amount::from_raw(raw_coins);
if let Err(err) = context.transfer_coins(Some(from_address), Some(to_address), coins, true)
// note: rights are not checked here we checked that to_address is an SC address above
// and we know that the sender is at the top of the call stack
if let Err(err) = context.transfer_coins(Some(from_address), Some(to_address), coins, false)
{
bail!(
"error transferring {} coins from {} to {}: {}",
Expand Down Expand Up @@ -740,19 +747,26 @@ impl Interface for InterfaceImpl {
if validity_end.1 >= self.config.thread_count {
bail!("validity end thread exceeds the configuration thread count")
}
let target_addr = Address::from_str(target_address)?;

// check that the target address is an SC address
if !target_addr.is_smart_contract() {
bail!("target address is not a smart contract address")
}

let mut execution_context = context_guard!(self);
let emission_slot = execution_context.slot;
let emission_index = execution_context.created_message_index;
let sender = execution_context.get_current_address()?;
let coins = Amount::from_raw(raw_coins);
let fee = Amount::from_raw(raw_fee);
execution_context.transfer_coins(Some(sender), None, coins, true)?;
let fee = Amount::from_raw(raw_fee);
execution_context.transfer_coins(Some(sender), None, fee, true)?;
execution_context.push_new_message(AsyncMessage::new_with_hash(
emission_slot,
emission_index,
sender,
Address::from_str(target_address)?,
target_addr,
target_handler.to_string(),
max_gas,
fee,
Expand Down

0 comments on commit 68db6bd

Please sign in to comment.