Description
Insufficient balance checks result in arithmetic underflow errors when balance is less than the withdraw amount.
Impact
The impact is low as there does not appear to be an impact on GAS and the balance uses a uint256 so it cannot become negative. However insufficient error handling can make debugging more difficult and should be considered a code smell.
Proof of Concept
This is the diff showing the change needed.
File:: Laundrette.sol
function withdrawMoney(
address account,
address to,
uint256 amount
)
external
onlyRole("gangmember")
isAuthorizedOrRevert(account)
{
+ // Without this check we will get an underflow error if the amount
+ // exceeds the amount in the account
+ require(moneyShelf.getAccountAmount(account) >= amount,
+ "Laundrette::withdrawMoney insufficient balance");
moneyShelf.withdrawUSDC(account, to, amount);
}
The following shows the error without the require check shown above.
vm.prank(member2);
laundrette.withdrawMoney(member2, member1, 2);
Output:
├─ [6795] Laundrette::withdrawMoney(member2: [0xbD405055317EF6089a771008453365060BB97aC1], member1: [0x86fEa303737F5219d7b5E4f30Cb7268Bec1775Fa], 2)
│ ├─ [707] Kernel::hasRole(member2: [0xbD405055317EF6089a771008453365060BB97aC1], 0x67616e676d656d62657200000000000000000000000000000000000000000000) [staticcall]
│ │ └─ ← [Return] true
│ ├─ [4437] MoneyShelf::withdrawUSDC(member2: [0xbD405055317EF6089a771008453365060BB97aC1], member1: [0x86fEa303737F5219d7b5E4f30Cb7268Bec1775Fa], 2)
│ │ ├─ [2917] Kernel::modulePermissions(0x4d4f4e4559000000000000000000000000000000000000000000000000000000, Laundrette: [0xD76ffbd1eFF76C510C3a509fE22864688aC3A588], 0x7922620900000000000000000000000000000000000000000000000000000000) [staticcall]
│ │ │ └─ ← [Return] true
│ │ └─ ← [Revert] panic: arithmetic underflow or overflow (0x11)
│ └─ ← [Revert] panic: arithmetic underflow or overflow (0x11)
└─ ← [Revert] panic: arithmetic underflow or overflow (0x11)
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 5.43ms (1.63ms CPU time)
[FAIL. Reason: panic: arithmetic underflow or overflow (0x11)] test_depositInternalAccountingBalance() (gas: 367795)
Recommended mitigation
References
N/A
Tools Used
