HACKED ADMIN OR MALICIOUS ADMIN CAN IMMEDIATELY STEAL ALL ASSETS ON THE PLATFORM

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-drips/blob/main/src/DripsHub.sol#L416 https://github.com/code-423n4/2023-01-drips/blob/main/src/DripsHub.sol#L533

Vulnerability details

Impact

When giving or setting Drips, the ERC20 tokens are transfered from the user to the DripsHub contract. Therefore, all assets managed by the platform are held by this contract. The requests are made here:

File: DripsHub.sol

L409:   function give(uint256 userId, uint256 receiver, IERC20 erc20, uint128 amt)
        public
        whenNotPaused
        onlyDriver(userId)
    {
        _increaseTotalBalance(erc20, amt);
        Splits._give(userId, receiver, _assetId(erc20), amt);
        erc20.safeTransferFrom(msg.sender, address(this), amt); // <-- Get tokens from the user (by the driver)
    } 

L510:   function setDrips(
        uint256 userId,
        IERC20 erc20,
        DripsReceiver[] memory currReceivers,
        int128 balanceDelta,
        DripsReceiver[] memory newReceivers,
        // slither-disable-next-line similar-names
        uint32 maxEndHint1,
        uint32 maxEndHint2
    ) public whenNotPaused onlyDriver(userId) returns (int128 realBalanceDelta) {
        if (balanceDelta > 0) {
            _increaseTotalBalance(erc20, uint128(balanceDelta));
        }
        realBalanceDelta = Drips._setDrips(
            userId,
            _assetId(erc20),
            currReceivers,
            balanceDelta,
            newReceivers,
            maxEndHint1,
            maxEndHint2
        );
        if (realBalanceDelta > 0) {
            erc20.safeTransferFrom(msg.sender, address(this), uint128(realBalanceDelta));   // <-- Get tokens from the user (by the driver)
        } else if (realBalanceDelta < 0) {
            _decreaseTotalBalance(erc20, uint128(-realBalanceDelta));
            erc20.safeTransfer(msg.sender, uint128(-realBalanceDelta));
        }
    }

DripsHub.sol#L416 DripsHub.sol#L533

The issue is that there is a significant centralization risk trusting DripsHub.sol contract to behave well, because it is an immediately upgradeable ERC1967Proxy.

Proof of Concept

All it takes for a malicious owner or hacked owner is to upgrade to the following contract:

function stealTokens(
    ERC20 token,
    address to
) external onlyAdmin {
    token.transfer(to, token.balanceOf(address(this)));
}

At this point hacker or admin can steal all the assets of DripsHub.sol (i.e., all assets on the platform).

Tools Used

Inspection.

Recommended Mitigation Steps

Managed.sol contract proxy should implement a timelock of several times cycleSecs, to give users enough time to withdraw their funds before some malicious action becomes possible.

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid

[GalloDaSballo]

Upgradeability is OOS

[c4-judge]

GalloDaSballo marked the issue as nullified

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Out of scope