Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

RaymondFam - Double-entry point (Two Address) token might raise some issues #416

Closed
github-actions bot opened this issue Feb 21, 2023 · 0 comments
Closed
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue

Comments

@github-actions
Copy link

github-actions bot commented Feb 21, 2023

RaymondFam

medium

Double-entry point (Two Address) token might raise some issues

Summary

Two address tokens exist in the blockchain, a token which has two addreses. For example, Synthetix’s ProxyERC20 contract is such a token which exists in many forms (sUSD, sBTC, ... ). TUSD, which famously caused a potential attack on Compound is another good example. Double-entry point (Two Address) token might raise some issues such as double spending and quicker reaching of openQTokenWhitelist.TOKEN_ADDRESS_LIMIT().

Vulnerability Detail

When addToken() is called by the owner in TokenWhitelist.sol, it only checks the token has not been whitelisted prior to whitelisting the token and incrementing tokenCount. There could be a scenario where the two addresses (of Double entry point token) are being registered successfully in two separate instances.

Impact

This will speed up TOKEN_ADDRESS_LIMIT being reached by taking up double slots. Additionally, it could lead to double execution when fundBountyToken() is called by the proxy in DepositManagerV1.sol.

Code Snippet

File: TokenWhitelist.sol#L25-L32

    function addToken(address _tokenAddress) external onlyOwner {
        require(
            !this.isWhitelisted(_tokenAddress),
            Errors.TOKEN_ALREADY_WHITELISTED
        );
        whitelist[_tokenAddress] = true;
        tokenCount++;
    }

File: DepositManagerV1.sol#L36-L74

    function fundBountyToken(
        address _bountyAddress,
        address _tokenAddress,
        uint256 _volume,
        uint256 _expiration,
        string memory funderUuid
    ) external payable onlyProxy {
        IBounty bounty = IBounty(payable(_bountyAddress));

        if (!isWhitelisted(_tokenAddress)) {
            require(
                !tokenAddressLimitReached(_bountyAddress),
                Errors.TOO_MANY_TOKEN_ADDRESSES
            );
        }

        require(bountyIsOpen(_bountyAddress), Errors.CONTRACT_ALREADY_CLOSED);

        (bytes32 depositId, uint256 volumeReceived) = bounty.receiveFunds{
            value: msg.value
        }(msg.sender, _tokenAddress, _volume, _expiration);

        bytes memory funderUuidBytes = abi.encode(funderUuid);

        emit TokenDepositReceived(
            depositId,
            _bountyAddress,
            bounty.bountyId(),
            bounty.organization(),
            _tokenAddress,
            block.timestamp,
            msg.sender,
            _expiration,
            volumeReceived,
            0,
            funderUuidBytes,
            VERSION_1
        );
    }

Tool used

Manual Review

Recommendation

Either redesign assets registration and limit any double entry point token if necessary by detecting a proxy pattern or balance checking when the asset is being transferred in order to prevent double spending (or transfer).

Duplicate of #530

@github-actions github-actions bot added Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue labels Feb 21, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Mar 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

1 participant