code-423n4 / 2023-01-drips-findings

0 stars 2 forks source link

Hacked or malicious owner can steal all tokens #289

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-drips/blob/9fd776b50f4be23ca038b1d0426e63a69c7a511d/src/Managed.sol#L157

Vulnerability details

Tokens for all active drips are stored in the DripsHub contract. Since DripsHub is an upgradeable ERC1967Proxy, a malicious or hacked owner can simply upgrade the contract to include e.g., the following function:

function stealTokens(IERC20 token, address to, uint256 amt) external {
    require(msg.sender == <hacker_address>);
    token.safeTransfer(to, amt);
}

and immediately steal all tokens in the contract. Additionally, all token approvals to the contract can be exploited.

Should implement a timelock to give users time to withdraw tokens and revoke approvals or remove upgradeability altogether.

GalloDaSballo commented 1 year ago

We cannot accept Upgradeability as a valid vulnerability because the in-scope code doesn't have that functionality

We assume that end users understand the risk of upgradeable contracts

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Out of scope