Skip to content

Mitigation Confirmed for M-01 #19

Description

@code423n4

Mitigation of M-01: Fully Alleviated

The sponsor implemented the recommended course of action to address this exhibit, introducing the notBlacklisted modifier to the KUMABondToken::approve call and thus preventing approvals to be performed on behalf of a blacklisted owner. A snippet of the contract with the remediated code showcased can be found below:

/**
 * @dev See {IERC721-approve}.
 * @dev Adds the following conditions to the call :
 *      - Caller and spender must not be blacklisted
 *      - Contract must not be paused
 */
function approve(address to, uint256 tokenId)
    public
    override(ERC721, IERC721)
    whenNotPaused
    notBlacklisted(to)
    notBlacklisted(msg.sender)

    /** 
     * MITIGATION BLOCK OF M-01 START
     */

    notBlacklisted(ownerOf(tokenId))

    /** 
     * MITIGATION BLOCK OF M-01 END
     */
{
    address owner = ERC721.ownerOf(tokenId);

    if (to == owner) {
        revert Errors.ERC721_APPROVAL_TO_CURRENT_OWNER();
    }
    if (_msgSender() != owner && !isApprovedForAll(owner, _msgSender())) {
        revert Errors.ERC721_APPROVE_CALLER_IS_NOT_TOKEN_OWNER_OR_APPROVED_FOR_ALL();
    }
    _approve(to, tokenId);
}

As such, it is no longer possible to approve an NFT asset on behalf of a blacklisted owner. As an optimization, I would recommend the blacklist validation to be performed within the function's body as presently, the evaluation of ERC721::ownerOf is performed twice redundantly (once for the modifier and once in the function's body).

An accompanying test was introduced to the codebase's KUMABondToken.t.sol file that ensures a BLACKLIST_ACCOUNT_IS_BLACKLISTED error is yielded with the correct address argument.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions