Upgraded Q -> 2 from #356 [1677633435546]

[c4-judge]

Judge has assessed an item in Issue #356 as 2 risk. The relevant finding follows:

[L-01] ERC4626 does not work with fee-on-transfer tokens in project Impact ERC20 token contract can be deposited with the deposit function.

With the following part of the code, the ERC20 transfer from msg.sender to the contract takes place; asset.safeTransferFrom(msg.sender, address(this), assets);

src/vault/Vault.sol: 134: function deposit(uint256 assets, address receiver) 135: public 136: nonReentrant 137: whenNotPaused 138: syncFeeCheckpoint 139: returns (uint256 shares) 140: { 141: if (receiver == address(0)) revert InvalidReceiver(); 142: 143: uint256 feeShares = convertToShares( 144: assets.mulDiv(uint256(fees.deposit), 1e18, Math.Rounding.Down) 145: ); 146: 147: shares = convertToShares(assets) - feeShares; 148: 149: if (feeShares > 0) _mint(feeRecipient, feeShares); 150: 151: _mint(receiver, shares); 152: 153: asset.safeTransferFrom(msg.sender, address(this), assets); 154: 155: adapter.deposit(assets, address(this)); 156: 157: emit Deposit(msg.sender, receiver, assets, shares); 158: } Proof Of Concept Contract calls transfer from contractA 100 tokens to current contract Current contract thinks it received 100 tokens It updates tokenBalances sheet +100 tokens While actually contract received only 90 tokens That breaks whole math for given token

Recommended Mitigation Steps There are several possible scenarios to prevent that.

Check how much contract balance is increased/decreased after every transfer (costs more gas) Make a separate contract that checks if the token has fee-on-transfer and store bool value depending on the result. If there is fee-on-transfer you can throw a require not allowing to use such token in the system while still saving lots of gas checking it only once. Or if you still want to allow fee-on-transfer tokens, amount variable must be updated to the balance difference after and before transfer.

This code block should be used if you want to ban fee-on-transfer tokens;

uint256 balanceBefore = IERC20(token).balanceOf(address(this)); IERC20(token). safeTransferFrom(msg.sender, address(this), amount); uint256 balanceAfter = IERC20(token).balanceOf(address(this)); require(balanceAfter - amount == balanceBefore, "feeOnTransfer token not supported");

[c4-judge]

dmvt marked the issue as duplicate of #503

[c4-judge]

dmvt marked the issue as satisfactory