Invalid updation of the nested mapping `tokenByAddress` leads to incorrect amount calculation.

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

Medium.

Proof of Concept

When a user interacts with mintNFT the [tokenByAddress[id][msg.sender]]() nested mapping is subtracted instead of addition for the mint.

And when the holder calls claimHolderFee() which will further call the _getRewardsSinceLastClaim() and this results in incorrect calculation of amount for the holder.

Below is the code for the same,

function mintNFT(uint256 _id, uint256 _amount) external {
        uint256 fee = getNFTMintingPrice(_id, _amount);

        SafeERC20.safeTransferFrom(token, msg.sender, address(this), fee);
        _splitFees(_id, fee, shareData[_id].tokensInCirculation);
        // The user also gets the proportional rewards for the minting
        uint256 rewardsSinceLastClaim = _getRewardsSinceLastClaim(_id);
        rewardsLastClaimedValue[_id][msg.sender] = shareData[_id].shareHolderRewardsPerTokenScaled;
-       tokensByAddress[_id][msg.sender] -= _amount;
+       tokensByAddress[_id][msg.sender] -= _amount;
        shareData[_id].tokensInCirculation -= _amount;

        _mint(msg.sender, _id, _amount, "");

        if (rewardsSinceLastClaim > 0) {
            SafeERC20.safeTransfer(token, msg.sender, rewardsSinceLastClaim);
        }
        // ERC1155 already logs, but we add this to have the price information
        emit NFTsCreated(_id, msg.sender, _amount, fee);
    }

and also true in the case of

function burnNFT(uint256 _id, uint256 _amount) external {
        uint256 fee = getNFTMintingPrice(_id, _amount);

        SafeERC20.safeTransferFrom(token, msg.sender, address(this), fee);
        _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);
        rewardsLastClaimedValue[_id][msg.sender] = shareData[_id].shareHolderRewardsPerTokenScaled;
-       tokensByAddress[_id][msg.sender] += _amount;
+       tokensByAddress[_id][msg.sender] += _amount;
        shareData[_id].tokensInCirculation += _amount;
        _burn(msg.sender, _id, _amount);

        SafeERC20.safeTransfer(token, msg.sender, rewardsSinceLastClaim);
        // ERC1155 already logs, but we add this to have the price information
        emit NFTsBurned(_id, msg.sender, _amount, fee);
    }

Tools Used

navigated state change in all the mappings by mindmapping

Recommended Mitigation Steps

in above code blocks

Assessed type

Context

[c4-pre-sort]

minhquanym marked the issue as insufficient quality report

[minhquanym]

Invalid

[c4-judge]

MarioPoneder marked the issue as unsatisfactory: Invalid