m4ttm - Approvals persist when a CouncilMember is burned and the id is later reminted

[sherlock-admin]

m4ttm

high

Approvals persist when a CouncilMember is burned and the id is later reminted

Summary

When a CouncilMember is burned, the approved address remains set and will remain set if the id is reminted.

Vulnerability Detail

Approvals are stored in the _tokenApproval mapping which is not cleared when a token is burned then reminted.

Impact

If a token is burned and assumed to be gone, then later reminted to a new user it is possible that the approved address may relate to the old user and be incorrect. This could potentially lead to another account being able to perform unauthorized actions on behalf of another user.

Code Snippet

https://github.com/sherlock-audit/2024-01-telcoin/blob/0954297f4fefac82d45a79c73f3a4b8eb25f10e9/telcoin-audit/contracts/sablier/core/CouncilMember.sol#L210-L222

function burn(
        uint256 tokenId,
        address recipient
    ) external onlyRole(GOVERNANCE_COUNCIL_ROLE) {
        require(totalSupply() > 1, "CouncilMember: must maintain council");
        _retrieve();
        _withdrawAll(recipient, tokenId);

        uint256 balance = balances[balances.length - 1];
        balances[tokenId] = balance;
        balances.pop();
        _burn(tokenId);
    }

Tool used

Manual Review

Recommendation

Clear the _tokenApproval mapping in the burn function

_tokenApproval[tokenId]=address(0);

Duplicate of #35

[sherlock-admin2]

2 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid because { No approval is being made as there is no need of it; the mapping is fro an mergency approval and not for regular users i believe}

takarez commented:

invalid because {no explanation by the watson}