supplyTokenTo doesn't account for safeTransferFrom fees

[code423n4]

Handle

gpersoon

Vulnerability details

Impact

The function supplyTokenTo of MStableYieldSource retrieves the tokens from the msg.sender and deposits them. However some tokens, like USDT might subtract a fee when transferring tokens. This means less tokens would be transferred than expected.

If this is not accounted for the MStableYieldSource contract would loose funds.

Note: other projects, like gro have special code for this situation: https://github.com/code-423n4/2021-06-gro/blob/main/contracts/DepositHandler.sol#L146

Proof of Concept

//https://github.com/pooltogether/pooltogether-mstable/blob/main/contracts/yield-source/MStableYieldSource.sol#L82 function supplyTokenTo(uint256 mAssetAmount, address to) external override nonReentrant { mAsset.safeTransferFrom(msg.sender, address(this), mAssetAmount); uint256 creditsIssued = savings.depositSavings(mAssetAmount); imBalances[to] += creditsIssued; emit Supplied(msg.sender, to, mAssetAmount); }

Tools Used

Recommended Mitigation Steps

Change the code to something like: function supplyTokenTo(uint256 mAssetAmount, address to) external override nonReentrant { uint256 mAssetBalanceBefore = mAsset.balanceOf(address(this)); // remember balance before mAsset.safeTransferFrom(msg.sender, address(this), mAssetAmount); uint256 mAssetBalanceAfter = mAsset.balanceOf(address(this)); // check balance after uint256 mAssetsActual = mAssetBalanceAfter - mAssetBalanceBefore; // calculate difference uint256 creditsIssued = savings.depositSavings(mAssetsActual); imBalances[to] += creditsIssued;

    emit Supplied(msg.sender, to, mAssetAmount); // perhaps update the event to include mAssetsActual 
}

[PierrickGT]

There is only a redemption and swap fee for mAssets, no fees on transfer so we don't need to implement this functionality.

Here is the documentation about fees: https://docs.mstable.org/mstable-assets/mstable-app/forge/minting-and-redemption/rebalancing#setting-fees

And you can see on Etherscan that there is only two redemptionFee and swapFee public variables: https://etherscan.io/address/0xe2f2a5C287993345a840Db3B0845fbC70f5935a5#readProxyContract

[0xean]

mAssets are not fee on transfer tokens so in the specific case outlined above, this is not needed. Closing.