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

0 stars 2 forks source link

Receivers with unregistered driver id can lead to tokens being stolen by hackers #239

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/DripsHub.sol#L409-L417 https://github.com/code-423n4/2023-01-drips/blob/9fd776b50f4be23ca038b1d0426e63a69c7a511d/src/DripsHub.sol#L510-L538 https://github.com/code-423n4/2023-01-drips/blob/9fd776b50f4be23ca038b1d0426e63a69c7a511d/src/DripsHub.sol#L576-L582

Vulnerability details

Impact

Users' tokens can be stolen if they provide receivers with unregistered driver id.

Proof of Concept

All of the functions DripsHub.sol#give(), DripsHub.sol#setDrips(), DripsHub.sol#setSplits() do not check if the driver ids of the receivers are registered or not. If the driver of a receiver has not been registered, tokens sending to it will not be able to collected.

A hacker can detect these uncollectable tokens by listening to the on-chain state, and register the unregistered driver id through calling DripsHub.sol#registerDriver() multiple times. After that, the hacker can collect the tokens using his own malicious driver contract.

Tools Used

Manual

Recommended Mitigation Steps

I recommend checking the driver ids of all receivers in the head of DripsHub.sol#give(), DripsHub.sol#setDrips(), DripsHub.sol#setSplits().

GalloDaSballo commented 1 year ago

Self-inflicted loss

Am thinking QA at best

xmxanuel commented 1 year ago

We see this as a bit similar to Ethereum addresses on Ethereum. You can send it to any address it might not exist or It maybe will be generated later.

Not an issue

CodeSandwich commented 1 year ago

[dispute validity] What Manuel said.

c4-sponsor commented 1 year ago

CodeSandwich marked the issue as sponsor disputed

c4-judge commented 1 year ago

GalloDaSballo changed the severity to QA (Quality Assurance)

GalloDaSballo commented 1 year ago

Per https://github.com/code-423n4/org/issues/53 downgrading to QA

L

c4-judge commented 1 year ago

GalloDaSballo marked the issue as grade-c