Proxy admin of DripsHub, AddressDriver, NFTDriver and ImmutableSplitsDriver can steal users' tokens by upgrading the contract

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-drips/blob/9fd776b50f4be23ca038b1d0426e63a69c7a511d/src/DripsHub.sol#L53 https://github.com/code-423n4/2023-01-drips/blob/9fd776b50f4be23ca038b1d0426e63a69c7a511d/src/AddressDriver.sol#L19 https://github.com/code-423n4/2023-01-drips/blob/9fd776b50f4be23ca038b1d0426e63a69c7a511d/src/NFTDriver.sol#L19 https://github.com/code-423n4/2023-01-drips/blob/9fd776b50f4be23ca038b1d0426e63a69c7a511d/src/ImmutableSplitsDriver.sol#L11 https://github.com/code-423n4/2023-01-drips/blob/9fd776b50f4be23ca038b1d0426e63a69c7a511d/src/Managed.sol#L157-L161

Vulnerability details

Impact

Proxy admin of DripsHub, AddressDriver, NFTDriver and ImmutableSplitsDriver can perform different malicious actions through upgrading, all can lead to users' assets being stolen.

Proof of Concept

An upgradable proxy contract can be upgraded with arbitrary functionality. This allows the admin of the proxy to perform malicious actions.

In order to use the drivers(AddressDriver, NFTDriver, and ImmutableSplitsDriver), users will allow them to spend their tokens. Therefore, the proxy admin can upgrade these drivers with a malicious contract to steal tokens from users' wallets, just need to call transferFrom of the tokens.

Most of users' tokens will be transferred to DripsHub for dripping. Therefore, the proxy admin can upgrade DripsHub with a malicious contract to steal tokens in it, just need to call transfer of the tokens.

Tools Used

Manual

Recommended Mitigation Steps

I recommend making these contracts un-upgradable.

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid

[c4-judge]

GalloDaSballo marked the issue as nullified

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid

[GalloDaSballo]

We cannot accept upgradeability as a vulnerability nor as an excuse for vulnerabilities