PhiNFT1155 are transferrable even after being paused

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-08-phi/blob/8c0985f7a10b231f916a51af5d506dd6b0c54120/src/art/PhiNFT1155.sol#L307
https://github.com/code-423n4/2024-08-phi/blob/8c0985f7a10b231f916a51af5d506dd6b0c54120/src/art/PhiNFT1155.sol#L332

Vulnerability details

Impact

PhiNFT1155.sol is pausable, and as expected, when tokens are paused, they shouldn't be transferrable. However, the safeTransferFrom and safeBatchTransferFrom functions are missing the whenNotPaused modifier.

Proof of Concept

PhiNFT1155.sol is pausable,

contract PhiNFT1155 is
//...
    PausableUpgradeable,
//...
{

and exposes the functions to pause and unpause the contracts.

    /// @notice Pauses the contract.
    function pause() external onlyOwner {
        _pause();
    }

    /// @notice Unpauses the contract.
    function unPause() external onlyOwner {
        _unpause();
    }

But when the token is paused, users can still transfer their tokens, defeating the purpose of its pausability. This is because the transfer functions are missing the whenNotPaused modifier.

    function safeTransferFrom(
        address from_,
        address to_,
        uint256 id_,
        uint256 value_,
        bytes memory data_
    )
        public
        override
    {
        if (from_ != address(0) && soulBounded(id_)) revert TokenNotTransferable();
        address sender = _msgSender();
        if (from_ != sender && !isApprovedForAll(from_, sender)) {
            revert ERC1155MissingApprovalForAll(sender, from_);
        }

        _safeTransferFrom(from_, to_, id_, value_, data_);
    }

    /// @notice Safely transfers multiple art tokens from one address to another.
    /// @param from_ The sender address.
    /// @param to_ The recipient address.
    /// @param ids_ The token IDs.
    /// @param values_ The amounts to transfer.
    /// @param data_ Additional data.
    function safeBatchTransferFrom(
        address from_,
        address to_,
        uint256[] memory ids_,
        uint256[] memory values_,
        bytes memory data_
    )
        public
        override
    {
        for (uint256 i; i < ids_.length; i++) {
            if (from_ != address(0) && soulBounded(ids_[i])) revert TokenNotTransferable();
        }
        address sender = _msgSender();
        if (from_ != sender && !isApprovedForAll(from_, sender)) {
            revert ERC1155MissingApprovalForAll(sender, from_);
        }
        _safeBatchTransferFrom(from_, to_, ids_, values_, data_);
    }

As a result, when the token is paused, it can still be transferred.

Tools Used

Manual Review

Recommended Mitigation Steps

Add the whenNotPaused modifier to the safeTransferFrom and safeBatchTransferFrom functions.

Assessed type

Token-Transfer