Skip to content
This repository was archived by the owner on Nov 6, 2020. It is now read-only.

Commit cc91e95

Browse files
kirushik5chdn
authored andcommitted
Additional error for invalid gas (#10327) (#10329)
* Tag sensible place (ECHECH) * Additional overflows checks.
1 parent 5d5b372 commit cc91e95

File tree

8 files changed

+37
-16
lines changed

8 files changed

+37
-16
lines changed

ethcore/evm/src/interpreter/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -571,10 +571,10 @@ impl<Cost: CostType> Interpreter<Cost> {
571571
let out_size = self.stack.pop_back();
572572

573573
// Add stipend (only CALL|CALLCODE when value > 0)
574-
let call_gas = call_gas + value.map_or_else(|| Cost::from(0), |val| match val.is_zero() {
574+
let call_gas = call_gas.overflow_add(value.map_or_else(|| Cost::from(0), |val| match val.is_zero() {
575575
false => Cost::from(ext.schedule().call_stipend),
576576
true => Cost::from(0),
577-
});
577+
})).0;
578578

579579
// Get sender & receive addresses, check if we have balance
580580
let (sender_address, receive_address, has_balance, call_type) = match instruction {

ethcore/light/src/transaction_queue.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ impl AccountTransactions {
9595
}
9696

9797
fn next_nonce(&self) -> U256 {
98-
self.current.last().map(|last| last.nonce + 1)
98+
self.current.last().map(|last| last.nonce.saturating_add(1.into()))
9999
.unwrap_or_else(|| *self.cur_nonce.value())
100100
}
101101

@@ -107,7 +107,7 @@ impl AccountTransactions {
107107
while let Some(tx) = self.future.remove(&next_nonce) {
108108
promoted.push(tx.hash);
109109
self.current.push(tx);
110-
next_nonce = next_nonce + 1;
110+
next_nonce = next_nonce.saturating_add(1.into());
111111
}
112112

113113
promoted

ethcore/src/executive.rs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ impl<'a> CallCreateExecutive<'a> {
311311
let prev_bal = state.balance(&params.address)?;
312312
if let ActionValue::Transfer(val) = params.value {
313313
state.sub_balance(&params.sender, &val, &mut substate.to_cleanup_mode(&schedule))?;
314-
state.new_contract(&params.address, val + prev_bal, nonce_offset)?;
314+
state.new_contract(&params.address, val.saturating_add(prev_bal), nonce_offset)?;
315315
} else {
316316
state.new_contract(&params.address, prev_bal, nonce_offset)?;
317317
}
@@ -1102,9 +1102,13 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> {
11021102
let refunded = cmp::min(refunds_bound, (t.gas - gas_left_prerefund) >> 1);
11031103
let gas_left = gas_left_prerefund + refunded;
11041104

1105-
let gas_used = t.gas - gas_left;
1106-
let refund_value = gas_left * t.gas_price;
1107-
let fees_value = gas_used * t.gas_price;
1105+
let gas_used = t.gas.saturating_sub(gas_left);
1106+
let (refund_value, overflow_1) = gas_left.overflowing_mul(t.gas_price);
1107+
let (fees_value, overflow_2) = gas_used.overflowing_mul(t.gas_price);
1108+
if overflow_1 || overflow_2 {
1109+
return Err(ExecutionError::TransactionMalformed("U256 Overflow".to_string()));
1110+
}
1111+
11081112

11091113
trace!("exec::finalize: t.gas={}, sstore_refunds={}, suicide_refunds={}, refunds_bound={}, gas_left_prerefund={}, refunded={}, gas_left={}, gas_used={}, refund_value={}, fees_value={}\n",
11101114
t.gas, sstore_refunds, suicide_refunds, refunds_bound, gas_left_prerefund, refunded, gas_left, gas_used, refund_value, fees_value);
@@ -1122,7 +1126,7 @@ impl<'a, B: 'a + StateBackend> Executive<'a, B> {
11221126
}
11231127

11241128
// perform garbage-collection
1125-
let min_balance = if schedule.kill_dust != CleanDustMode::Off { Some(U256::from(schedule.tx_gas) * t.gas_price) } else { None };
1129+
let min_balance = if schedule.kill_dust != CleanDustMode::Off { Some(U256::from(schedule.tx_gas).overflowing_mul(t.gas_price).0) } else { None };
11261130
self.state.kill_garbage(&substate.touched, schedule.kill_empty, &min_balance, schedule.kill_dust == CleanDustMode::WithCodeAndStorage)?;
11271131

11281132
match result {

ethcore/src/state/account.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -462,12 +462,12 @@ impl Account {
462462

463463
/// Increment the nonce of the account by one.
464464
pub fn inc_nonce(&mut self) {
465-
self.nonce = self.nonce + U256::from(1u8);
465+
self.nonce = self.nonce.saturating_add(U256::from(1u8));
466466
}
467467

468468
/// Increase account balance.
469469
pub fn add_balance(&mut self, x: &U256) {
470-
self.balance = self.balance + *x;
470+
self.balance = self.balance.saturating_add(*x);
471471
}
472472

473473
/// Decrease account balance.

ethcore/src/state/mod.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,12 @@ impl<B: Backend> State<B> {
500500
/// it will have its code reset, ready for `init_code()`.
501501
pub fn new_contract(&mut self, contract: &Address, balance: U256, nonce_offset: U256) -> TrieResult<()> {
502502
let original_storage_root = self.original_storage_root(contract)?;
503-
self.insert_cache(contract, AccountEntry::new_dirty(Some(Account::new_contract(balance, self.account_start_nonce + nonce_offset, original_storage_root))));
503+
let (nonce, overflow) = self.account_start_nonce.overflowing_add(nonce_offset);
504+
if overflow {
505+
return Err(Box::new(TrieError::DecoderError(H256::from(contract),
506+
rlp::DecoderError::Custom("Nonce overflow".into()))));
507+
}
508+
self.insert_cache(contract, AccountEntry::new_dirty(Some(Account::new_contract(balance, nonce, original_storage_root))));
504509
Ok(())
505510
}
506511

miner/src/pool/queue.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,7 @@ impl TransactionQueue {
474474

475475
self.pool.read().pending_from_sender(state_readiness, address)
476476
.last()
477-
.map(|tx| tx.signed().nonce + 1)
477+
.map(|tx| tx.signed().nonce.saturating_add(U256::from(1)))
478478
}
479479

480480
/// Retrieve a transaction from the pool.

miner/src/pool/ready.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ impl<C: NonceClient> txpool::Ready<VerifiedTransaction> for State<C> {
9595
},
9696
cmp::Ordering::Less => txpool::Readiness::Stale,
9797
cmp::Ordering::Equal => {
98-
*nonce = *nonce + 1;
98+
*nonce = nonce.saturating_add(U256::from(1));
9999
txpool::Readiness::Ready
100100
},
101101
}
@@ -159,7 +159,7 @@ impl<C: Fn(&Address) -> Option<U256>> txpool::Ready<VerifiedTransaction> for Opt
159159
cmp::Ordering::Greater => txpool::Readiness::Future,
160160
cmp::Ordering::Less => txpool::Readiness::Stale,
161161
cmp::Ordering::Equal => {
162-
*nonce = *nonce + 1;
162+
*nonce = nonce.saturating_add(U256::from(1));
163163
txpool::Readiness::Ready
164164
},
165165
}

miner/src/pool/verifier.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,19 @@ impl<C: Client> txpool::Verifier<Transaction> for Verifier<C, ::pool::scoring::N
283283
}
284284
}
285285

286-
let cost = transaction.value + transaction.gas_price * transaction.gas;
286+
let (full_gas_price, overflow_1) = transaction.gas_price.overflowing_mul(transaction.gas);
287+
let (cost, overflow_2) = transaction.value.overflowing_add(full_gas_price);
288+
if overflow_1 || overflow_2 {
289+
trace!(
290+
target: "txqueue",
291+
"[{:?}] Rejected tx, price overflow",
292+
hash
293+
);
294+
bail!(transaction::Error::InsufficientBalance {
295+
cost: U256::max_value(),
296+
balance: account_details.balance,
297+
});
298+
}
287299
if account_details.balance < cost {
288300
debug!(
289301
target: "txqueue",

0 commit comments

Comments
 (0)