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

1 stars 1 forks source link

Consistently check account balance before and after transfers for Fee-On-Transfer discrepancies #153

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/wfCashLogic.sol#L84

Vulnerability details

As arbitrary ERC20 tokens can be passed, the amount here should be calculated every time to take into consideration a possible fee-on-transfer or deflation. Also, it's a good practice for the future of the solution.

Affected code:

File: wfCashLogic.sol
50:     function _mintInternal(
...
56:     ) internal nonReentrant {
...
68:         if (isETH) {
...
82:         } else {
83:             // Transfers tokens in for lending, Notional will transfer from this contract.
84:             token.safeTransferFrom(msg.sender, address(this), depositAmountExternal); //@audit fot incompatible
85: 
86:             // Executes a lending action on Notional
87:             BatchLend[] memory action = EncodeDecode.encodeLendTrade(
88:                 getCurrencyId(),
89:                 getMarketIndex(),
90:                 fCashAmount,
91:                 minImpliedRate,
92:                 useUnderlying
93:             );
94:             NotionalV2.batchLend(address(this), action);
95:         }

Recommended Mitigation Steps

Use the balance before and after the transfer to calculate the received amount instead of assuming that it would be equal to the amount passed as a parameter.

jeffywu commented 2 years ago

Duplicate #139

gzeoneth commented 2 years ago

Consider with #150