From 0d284c01958d30c8fd3298ed50f7f98a417bb9bc Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Thu, 27 Sep 2018 19:13:14 +0100 Subject: [PATCH] Fix transfer overflow exploit. (#824) --- substrate/srml/balances/src/lib.rs | 5 ++++- substrate/srml/balances/src/tests.rs | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/substrate/srml/balances/src/lib.rs b/substrate/srml/balances/src/lib.rs index 2f011ef270..ace960abc4 100644 --- a/substrate/srml/balances/src/lib.rs +++ b/substrate/srml/balances/src/lib.rs @@ -287,7 +287,10 @@ impl Module { let to_balance = Self::free_balance(&dest); let would_create = to_balance.is_zero(); let fee = if would_create { Self::creation_fee() } else { Self::transfer_fee() }; - let liability = value + fee; + let liability = match value.checked_add(&fee) { + Some(l) => l, + None => return Err("got overflow after adding a fee to value"), + }; let new_from_balance = match from_balance.checked_sub(&liability) { Some(b) => b, diff --git a/substrate/srml/balances/src/tests.rs b/substrate/srml/balances/src/tests.rs index 81de663377..9b3376b31d 100644 --- a/substrate/srml/balances/src/tests.rs +++ b/substrate/srml/balances/src/tests.rs @@ -471,3 +471,19 @@ fn account_removal_on_free_too_low() { }, ); } + +#[test] +fn transfer_overflow_isnt_exploitable() { + with_externalities( + &mut ExtBuilder::default().creation_fee(50).build(), + || { + // Craft a value that will overflow if summed with `creation_fee`. + let evil_value = u64::max_value() - 49; + + assert_err!( + Balances::transfer(Some(1).into(), 5.into(), evil_value), + "got overflow after adding a fee to value" + ); + } + ); +}