Guild token can be transferred at any time as there is no logic stopping it

[c4-bot-8]

Lines of code

https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/tokens/GuildToken.sol#L182-L191 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/tokens/GuildToken.sol#L288-L300 https://github.com/code-423n4/2023-12-ethereumcreditguild/blob/2376d9af792584e3d15ec9c32578daa33bb56b43/src/tokens/GuildToken.sol#L302-L315

Vulnerability details

According to the docs, the Guild token is not transferable at lunch the function _beforeTokenTransfer() is used to keep track of transfers and enforce this law.

    function _beforeTokenTransfer(
        address from,
        address to,
        uint256 /* amount*/
    ) internal view override {
        require(
            transferable || from == address(0) || to == address(0),
            "GuildToken: transfers disabled"
        );
    }

But this function is not called in the transfer function to check if transfers are allowed.

    function transfer(
        address to,
        uint256 amount
    )
        public
        virtual
        override(ERC20, ERC20Gauges, ERC20MultiVotes)
        returns (bool)
    {
        _decrementWeightUntilFree(msg.sender, amount);
        _decrementVotesUntilFree(msg.sender, amount);
        return ERC20.transfer(to, amount);
    }

    function transferFrom(
        address from,
        address to,
        uint256 amount
    )
        public
        virtual
        override(ERC20, ERC20Gauges, ERC20MultiVotes)
        returns (bool)
    {
        _decrementWeightUntilFree(from, amount);
        _decrementVotesUntilFree(from, amount);
        return ERC20.transferFrom(from, to, amount);
    }

Impact

Guild tokens are transferable at any time which goes against the intention of the developers

Tools Used

Manual Review

Recommended Mitigation Steps

The _beforeTokenTransfer() should be called in the transfer functions to regulate transfers.

Assessed type

ERC20

[0xSorryNotSorry]

It's already called as the function uses override keyword;

Contract: GuildToken.sol

182:     function _beforeTokenTransfer(
183:         address from,
184:         address to,
185:         uint256 /* amount*/
186:     ) internal view override {
187:         require(
188:             transferable || from == address(0) || to == address(0),
189:             "GuildToken: transfers disabled"
190:         );
191:     }

[c4-pre-sort]

0xSorryNotSorry marked the issue as insufficient quality report

[c4-judge]

Trumpero marked the issue as unsatisfactory: Invalid