USDC is upgradeable: received amount should be calculated

[code423n4]

Handle

Dravee

Vulnerability details

Impact

The idea came from reading this article: https://blog.coinbase.com/usdc-v2-upgrading-a-multi-billion-dollar-erc-20-token-b57cd9437096

As USDC is upgradeable, the received amount in the solution should be calculated everytime to take into consideration a possible transfer-on-fee or deflation.

Proof of Concept

Impacted places:

contracts\managers\SherlockClaimManager.sol:420:    TOKEN.safeTransferFrom(msg.sender, address(this), _amount);
contracts\managers\SherlockProtocolManager.sol:767:    token.safeTransferFrom(msg.sender, address(this), _amount);
contracts\SherBuy.sol:163:    usdc.safeTransferFrom(msg.sender, address(this), stake);
contracts\SherBuy.sol:165:    usdc.safeTransferFrom(msg.sender, receiver, price);
contracts\Sherlock.sol:539:    token.safeTransferFrom(msg.sender, address(this), _amount);

Tools Used

VS Code

Recommended Mitigation Steps

Get the actual received amount by calculating the difference of token balance before and after the transfer. The code should be similar to what's done in Sherlock.sol:_stake():

File: Sherlock.sol
421:     // Checks this amount of SHER tokens in this contract before we transfer new ones
422:     uint256 before = sher.balanceOf(address(this));
423: 
424:     // pullReward() calcs then actually transfers the SHER tokens to this contract
425:     // in case this call fails, whole (re)staking transaction fails
426:     _sher = sherDistributionManager.pullReward(_amount, _period, _id, _receiver);
427: 
428:     // actualAmount should represent the amount of SHER tokens transferred to this contract for the current stake position
429:     uint256 actualAmount = sher.balanceOf(address(this)) - before;

[Evert0x]

Dispute. If we want to create code takes into account every possible update we are not able to create any code at all. Or is there any roadmap for future USDC updates?

[jack-the-pug]

I will downgrade this to a Non-critical.