Skip to content

Latest commit

 

History

History
107 lines (74 loc) · 4.63 KB

043.md

File metadata and controls

107 lines (74 loc) · 4.63 KB
Raw

Interesting Brick Yeti

Medium

BLACKLISTER_ROLE can withdraw funds which are meant to be rescued by SUPPORT_ROLE

Summary

https://github.com/sherlock-audit/2024-11-telcoin/blob/b9c751b59e78a7123a636e31ecafc9147046f190/telcoin-audit/contracts/util/abstract/Blacklist.sol#L77-L85

The Blacklist::addBlackList() function doesn't check if the provided user is the Stablecoin contract which will cause to bypass SUPPORT_ROLE and withdraw big portion of the accidentally sent tokens to the Stablecoin contract.

Root Cause

If the BLACKLISTER_ROLE provides the Stablecoin's contract address as a function parameter accidentally (it is possible and a valid case according to the project's README) or if it is malicious, the Stablecoin native tokens which are accidentally sent to the Stablecoin contract will be withdrawn to the BLACKLISTER_ROLE address.

Blacklist.sol

function addBlackList(
    address user
) public virtual onlyRole(BLACKLISTER_ROLE) {
    if (blacklisted(user)) revert AlreadyBlacklisted(user);
    _onceBlacklisted(user);
    // do this after so it doesnt trip blacklisted restirctions
    _setBlacklist(user, true);
    emit AddedBlacklist(user);
}
Stablecoin.sol

function _onceBlacklisted(address user) internal override {
    _transfer(user, _msgSender(), balanceOf(user));
}

If the SUPPORT_ROLE tries to rescue the funds, the Stablecoin::erc20Rescue() function will revert, because the address is blacklisted. Even the BLACKLISTER_ROLE calls removeBlackList() to unblacklist the user, the tokens will be already sent.

The BLACKLISTER_ROLE uses the functionality which is meant to be used from SUPPORT_ROLE.

Internal pre-conditions

A user/s needs to accidentally sent Stablecoin tokens to the Stablecoin contract. It is very likely to happen and there are many real world examples. Big percentage of the accidentally sent tokens are the contract's own tokens. Some examples - OMG, WETH (Example1, Example2), etc.

External pre-conditions

No response

Attack Path

  1. A user accidentally sends 100 Stablecoin tokens to the Stablecoin contract
  2. The MINTER_ROLE calls addBlackList() function with Stablecoin's address as a parameter which transfers all the Stablecoin tokens of the Stablecoin contract to his address
  3. SUPPORT_ROLE sees there are tokens sent to the contract and tries to transfer them back to the user
  4. The erc20Rescue() function reverts, because the Stablecoin contract's address is blacklisted

Impact

The BLACKLISTER_ROLE uses SUPPORT_ROLE's functionality and it is possible the user, who accidentally sent tokens to the Stablecoin contract, doesn't get his tokens if the BLACKLISTER_ROLE is a smart contract and doesn't have a functionality to send back tokens.

PoC

Add the following test case in Stablecoin.test.ts

describe.only("Call addBlackList with Stablecoin's address", () => {
    it("blacklist", async () => {
        const users = await ethers.getSigners();
        let randomUser = users[2];
        let blacklister = users[3];
        let support = users[3];

        await stablecoin.grantRole(MINTER_ROLE, deployer);
        await stablecoin.grantRole(BLACKLISTER_ROLE, blacklister);
        await stablecoin.grantRole(SUPPORT_ROLE, support);

        // A random user has 100 Stablecoin tokens
        await stablecoin.mintTo(randomUser, 100);

        // The user accidentally sends the tokens to the contract
        await stablecoin.connect(randomUser).transfer(stablecoin, 100);

        // The minter calls addBlackList with Stablecoin's address as a user which transfers all the Stablecoin tokens to his address
        await stablecoin.connect(blacklister).addBlackList(stablecoin);

        // Support sees the tokens are sent to the contract and tries to transfer them back to the user
        await expect(stablecoin.connect(support).erc20Rescue(stablecoin, randomUser, 100)).to.be.reverted;

        expect(await stablecoin.balanceOf(blacklister)).to.equal(100);
        expect(await stablecoin.balanceOf(randomUser)).to.equal(0);
        expect(await stablecoin.balanceOf(stablecoin)).to.equal(0);
    });
});

Mitigation

Check if the user in _onceBlacklisted is the Stablecoin contract.

function _onceBlacklisted(address user) internal override {
+    require(user != address(this), "user can't be Stablecoin contract");
    _transfer(user, _msgSender(), balanceOf(user));
}