User may not receive token when claim fees

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-11-canto/blob/335930cd53cf9a137504a57f1215be52c6d67cb3/1155tech-contracts/src/Market.sol#L263

Vulnerability details

Impact

User may not receive token when claim fees, resulting in loss of user funds. The three claim fee functions in the Market: claimHolderFee claimCreatorFee claimPlatformFee, they all have the same problem.

Proof of Concept

In general, defi protocols can specify recipients when claiming fees, because msg.sender may not be able to accept the token

The reasons are as follows:

1. Some tokens have the blacklist function. msg.sender is added to the blacklist. Note that from here: https://github.com/d-xo/weird-erc20#tokens-with-blocklists we can see that USDC employs a blocklisting feature.
2. msg.sender is a contract account, if the contract code is not updatable, msg.sender may not be able to send the token.

Market#claimHolderFee use msg.sender as the recipient:

    function claimHolderFee(uint256 _id) external {
        uint256 amount = _getRewardsSinceLastClaim(_id);
        rewardsLastClaimedValue[_id][msg.sender] = shareData[_id].shareHolderRewardsPerTokenScaled;
        if (amount > 0) {
@>         SafeERC20.safeTransfer(token, msg.sender, amount);
        }
        emit HolderFeeClaimed(msg.sender, _id, amount);
    }

claimCreatorFee claimPlatformFee also uses msg.sender as the receiver.

Tools Used

vscode manual

Recommended Mitigation Steps

-    function claimHolderFee(uint256 _id) external {
+    function claimHolderFee(uint256 _id,address receiver) external {
        uint256 amount = _getRewardsSinceLastClaim(_id);
        rewardsLastClaimedValue[_id][msg.sender] = shareData[_id].shareHolderRewardsPerTokenScaled;
        if (amount > 0) {
-           SafeERC20.safeTransfer(token, msg.sender, amount);
+           SafeERC20.safeTransfer(token, receiver, amount);
        }
        emit HolderFeeClaimed(msg.sender, _id, amount);
    }

Assessed type

Other

[c4-pre-sort]

minhquanym marked the issue as insufficient quality report

[minhquanym]

Consider QA

[c4-judge]

MarioPoneder changed the severity to QA (Quality Assurance)

[c4-judge]

MarioPoneder marked the issue as grade-b