Users will benefit from fee spit when calling burnNFT() agains intended design.

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-11-canto/blob/b78bfdbf329ba9055ba24bd710c7e1c60251039a/1155tech-contracts/src/Market.sol#L230-L232

Vulnerability details

Impact

Users will benefit from fee spit when calling burnNFT() agains intended design.

Proof of Concept

Let us first take a look at how a user was prevented from benefiting from fee spit in buy() function

        // The reward calculation has to use the old rewards value (pre fee-split) to not include the fees of this buy
        // The rewardsLastClaimedValue then needs to be updated with the new value such that the user cannot claim fees of this buy
        uint256 rewardsSinceLastClaim = _getRewardsSinceLastClaim(_id);
        // Split the fee among holder, creator and platform
        _splitFees(_id, fee, shareData[_id].tokensInCirculation);
        rewardsLastClaimedValue[_id][msg.sender] = shareData[_id].shareHolderRewardsPerTokenScaled;

From the snippet above, note that rewardsSinceLastClaim was first obtained using the previous state before calling splitFees() so that the caller does not benefit from its fee, and rewardsLastClaimedValue[_id][msg.sender] was set to equal the updated value after calling _splitFees() so then no value will be claimable by the user when calling claimHolderFee(), this was done correctly.
However, in Market.buyNFT(), the proper order was not followed, splitFees() was called before setting
rewardsSinceLastClaim, which implies that user will benefit from the fee split.

 _splitFees(_id, fee, shareData[_id].tokensInCirculation);
        // The user does not get the proportional rewards for the burning (unless they have additional tokens that are not in the NFT)
        uint256 rewardsSinceLastClaim = _getRewardsSinceLastClaim(_id);

Tools Used

Manual review.

Recommended Mitigation Steps

// The user does not get the proportional rewards for the burning (unless they have additional tokens that are not in the NFT)
 uint256 rewardsSinceLastClaim = _getRewardsSinceLastClaim(_id);
 _splitFees(_id, fee, shareData[_id].tokensInCirculation);

Assessed type

Context

[c4-pre-sort]

minhquanym marked the issue as insufficient quality report

[minhquanym]

Invalid

[c4-pre-sort]

minhquanym marked the issue as primary issue

[c4-judge]

MarioPoneder marked the issue as unsatisfactory:
Insufficient quality