Skip to content

Validate Permission Before BatchWithdraw  #130

Open
@Mr-i-me-pontte

Description

Hey guys, im new to the whole debugging .Sol so forgive me if im wrong rsrsr :)
but was checking this contract's code today and got the feeling that the code could be incorrectly allowing the ChildChainManager to withdraw funds on behalf of users
without their permission. The issue is in the deposit function, where the withdrawnTokens mapping is being set to false
for each deposited token, but this value is never checked again before the tokens are withdrawn in the withdrawBatch
function.

To fix this issue, the withdrawBatch function should check the withdrawnTokens mapping before allowing the tokens to
be withdrawn. For example, the code could be changed to this:

    function withdrawBatch(uint256[] calldata tokenIds) external {
        require(_msgSender() == _getCaller(), "ChildMintableERC721: INVALID_CALLER");

        // limit batching of tokens due to gas limit restrictions
        require(tokenIds.length <= BATCH_LIMIT, "ChildMintableERC721: BATCH_LIMIT_EXCEEDED");

        for (uint256 i = 0; i < tokenIds.length; i++) {
            uint256 tokenId = tokenIds[i];
            require(_hasToken(tokenId), "ChildMintableERC721: TOKEN_NOT_FOUND");

            // check if the token has already been withdrawn
            require(!withdrawnTokens[tokenId], "ChildMintableERC721: TOKEN_ALREADY_WITHDRAWN");

            withdrawnTokens[tokenId] = true;
            _burn(tokenId);
        }

        emit WithdrawnBatch(_msgSender(), tokenIds);
    }

best of Luck to Yall :)

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions