Skip to content

Commit

Permalink
[stable2407] Backport #7365 (#7380)
Browse files Browse the repository at this point in the history
Backport #7365 into `stable2407` from vgantchev.

See the
[documentation](https://github.com/paritytech/polkadot-sdk/blob/master/docs/BACKPORT.md)
on how to use this bot.

<!--
  # To be used by other automation, do not modify:
  original-pr-number: #${pull_number}
-->

Co-authored-by: Valery Gantchev <v@lery.dev>
  • Loading branch information
1 parent 39e71bd commit fd4b790
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 2 deletions.
12 changes: 12 additions & 0 deletions prdoc/pr_7365.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
title: Use checked math in frame-balances named_reserve
doc:
- audience: Runtime Dev
description: |-
This PR modifies `named_reserve()` in frame-balances to use checked math instead of defensive saturating math.

The use of saturating math relies on the assumption that the value will always fit in `u128::MAX`. However, there is nothing preventing the implementing pallet from passing a larger value which overflows. This can happen if the implementing pallet does not validate user input and instead relies on `named_reserve()` to return an error (this saves an additional read)

This is not a security concern, as the method will subsequently return an error thanks to `<Self as ReservableCurrency<_>>::reserve(who, value)?;`. However, the `defensive_saturating_add` will panic in `--all-features`, creating false positive crashes in fuzzing operations.
crates:
- name: pallet-balances
bump: patch
6 changes: 4 additions & 2 deletions substrate/frame/balances/src/impl_currency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -651,8 +651,10 @@ where
Reserves::<T, I>::try_mutate(who, |reserves| -> DispatchResult {
match reserves.binary_search_by_key(id, |data| data.id) {
Ok(index) => {
// this add can't overflow but just to be defensive.
reserves[index].amount = reserves[index].amount.defensive_saturating_add(value);
reserves[index].amount = reserves[index]
.amount
.checked_add(&value)
.ok_or(ArithmeticError::Overflow)?;
},
Err(index) => {
reserves
Expand Down

0 comments on commit fd4b790

Please sign in to comment.