search

code-423n4 / 2022-06-notional-coop-findings

1 stars 1 forks source link

Deposit and mint function will be rendered useless for users who are depositing using eth since balances will never be finalised #216

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L172 https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L181

Vulnerability details

The penultimate function in NotionalV2.batchBalanceAndTradeAction() calculates the withdrawals and finalises the balance as mentioned :

https://github.com/notional-finance/contracts-v2/blob/feature/batch-lend-improved-calculations/contracts/external/actions/BatchAction.sol#L414-L416

but the following calculation will revert on an underflow since balanceState.netAssetTransferInternalPrecision is set to 0 in loadBalanceState() and never modified

https://github.com/notional-finance/contracts-v2/blob/905d68e06254a8b3e34e696e83b0f18468fc792c/contracts/internal/balances/BalanceHandler.sol#L512

If the netTransferAssetInternalPrecision will never be adjusted then switch the operands with the subtrahend (withdrawAmount) being replaced as the minuend and vice-a-versa.

Or in the case of batchLend(), the balanceState.netAssetTransferInternalPrecision should be set similarly to :

https://github.com/notional-finance/contracts-v2/blob/feature/batch-lend-improved-calculations/contracts/internal/balances/BalanceHandler.sol#L73-L75

So after the call to _executeDepositAction() in batchBalanceAndTradeAction (), the balanceState.netAssetTransferInternalPrecision should be set after this returned value :

https://github.com/notional-finance/contracts-v2/blob/905d68e06254a8b3e34e696e83b0f18468fc792c/contracts/external/actions/BatchAction.sol#L326

It also affects _sellfCash() :

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashLogic.sol#L292

as within the call to batchBalanceAndTradeAction (), the deposit actions are skipped:

https://github.com/notional-finance/contracts-v2/blob/905d68e06254a8b3e34e696e83b0f18468fc792c/contracts/external/actions/BatchAction.sol#L309

so balanceState.netAssetTransferInternalPrecision is still zero. So,whatever non-zero value that is to be subtracted to return the withdrawAmount will not be possible.

jeffywu commented 2 years ago

I don't really understand the terminology in the report, but we have tests that demonstrate the ability to deposit and redeem ETH: https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/tests/test_wrapped_fcash.py#L693