User is able to give allowance of tokens while Contract UbiquityDollarToken.sol is paused

High Severity

Summary

User is able to successfully give allowance while contract UbiquityDollarToken is paused,and when contract is unpaused able to transfer approved tokens(which were approved in state of pause) to another user.

Vulnerability Detail

UbiquityDollarToken.sol inherits from ERC20Ubiquity.sol and ERC20Ubiquity.sol inherits from ERC20Upgradeable.sol(openzepplin's contract).

Here, ERC20Ubiquity.sol doesn't override _approve() method which should have been overridden with modifier attached to it called whenNotPaused like they did with _transfer() method.

Here is the test in Foundry which proves this behavior

 //approving 100 tokens while UbiquityDollarToken paused,still test doesn't revert
 //also able to transfer approved tokens(which were approved in state of pause)amount from user1Address to another user2Address

    function testAllowance_NotReverts_WhenPaused() public {
        //created 2 user to transfer token
        address user1Address = makeAddr("user1");
        address user2Address = makeAddr("user2");

        //created pauser address,responsible for pausing
        address pauser = makeAddr("pauser");

        //here admin is granting pauser role
        vm.prank(admin);
        accessControlFacet.grantRole(keccak256("PAUSER_ROLE"), pauser);

        //here admin is minting 100 token to user user1Address
        vm.prank(admin);
        dollarToken.mint(user1Address, 100);

        //here pauser is pausing UbiquityDollarToken contract
        vm.prank(pauser);
        dollarToken.pause();

        //here while contract is paused user1Address is giving 100 tokens allowance to userAddress2
        vm.prank(user1Address);
        dollarToken.approve(user2Address, 100);

        //here confirming that user2Address has been granted allowance of 100 tokens while contract was paused
        assertEq(dollarToken.allowance(user1Address, user2Address), 100);

        // //here pauser is unPausing UbiquityDollarToken contract
        vm.prank(pauser);
        dollarToken.unpause();

        //here user2Address is transferring tokens(which were approved in state of pause).
        vm.prank(user2Address);
        dollarToken.transferFrom(user1Address, user2Address, 100);

        //it is asserted here that allowance token transfer were a success which were approved while contract is pasued
        assertEq(dollarToken.balanceOf(user1Address), 0);
        assertEq(dollarToken.balanceOf(user2Address), 100);
    }

Impact

A malicious can take advantage of this meanwhile contract is exploited and take access of funds from affected wallets and when contract is unpaused he can successfully transfer those funds to itself. Pausing a contract can be a safety measure during potential vulnerabilities or attacks. Allowing allowances during this time might risk unauthorized access or transfers, defeating the purpose of the pause and compromising security. It might also cause confusion or distrust among users regarding the protocol's reliability.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/core/UbiquityDollarToken.sol#L13 https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/core/ERC20Ubiquity.sol#L21 https://github.com/sherlock-audit/2023-12-ubiquity/blob/d9c39e8dfd5601e7e8db2e4b3390e7d8dff42a8e/ubiquity-dollar/packages/contracts/src/dollar/core/ERC20Ubiquity.sol#L8C81-L8C97

Tool used

Foundry

Recommendation

Here,In ERC20Ubiquity.sol should have overridden _approve method which should have modifier attached to it called whenNotPaused like they did with _transfer() method.

sherlock-admin2 commented 7 months ago

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

auditsea commented:

Makes no effect on protocol