Closed c4-bot-7 closed 9 months ago
raymondfam marked the issue as sufficient quality report
raymondfam marked the issue as duplicate of #172
alcueca changed the severity to 2 (Med Risk)
alcueca marked the issue as satisfactory
alcueca changed the severity to 2 (Med Risk)
alcueca changed the severity to 3 (High Risk)
Lines of code
https://github.com/code-423n4/2024-01-curves/blob/main/contracts/Curves.sol#L155-L160 https://github.com/code-423n4/2024-01-curves/blob/main/contracts/Curves.sol#L240-L243
Vulnerability details
Any Curves token subject can selectively deny any buyings or sellings of their own token
Summary
Due to the referral mechanism, any Curves token subject can selectively deny any buyings or sellings with them being the token subject.
This attack is also possible when referral fee is zero.
Vulnerability details
The problem here is the referral mechanism itself. The attack is made possible due to two facts:
referralFeeDestination
, at any time, without limits._transferFees()
is triggered, which makes an external call toreferralFeeDestination
The
referralFeeDestination
functionWe examine the
referralFeeDestination()
functionIt is clear here that the token subject can set their own referral address at any time.
The
_transferFees()
external callThe function
_transferFees()
is triggered any time a token purchase or sell happens. The dangerous external call is as follow:If the referral fee destination transfer fails, the entire tx will fail. Since a token subject can freely set
referralFeeDestination
, they can also freely control whether the buy/sell tx will go through.While there are two external calls in the function (to the token subject, and to the referral), we show in the next section that this attack is only possible using the referral, and not the token subject.
Proof of concept
referralFeeDestination
to an empty contract (or any contract she has control over).For example, Alice can now only allow her close friends to sell tokens for a high price (for example, using a whitelist contract for referral), and lock all other user's funds.
receive()
function by, for example, checking the balance of every friends, and see whose balance changed (if any).Note that Alice's address herself is not feasible as an attack vector despite still being a fund recipient. This is because Alice must be the first to buy her own token, and must buy one token (an "initialization" step). Then it would be the user's fault to buy a malicious contract's token to begin with.
The higher impact is that, any user can decide to turn malicious at any time should they discover this attack vector. This essentially sets a ticking bomb on the protocol, with the TVL being held hostage.
Coded PoC
The PoC consists of two files, a mock malicious contract, and the test itself.
First, paste the following contract into
\contracts\Test
Then, paste the entire following test file into
\test
. Finally run test the normally withyarn test
. The test will fail withCannotSendFunds()
.Impact
Anyone can selectively deny any buyings or sellings with them being the token subject. This can also be used to prevent any of their own popularity decline, whitelisting only a group of people to sell tokens including themselves, or extend into a honeypot attack vector, or simply to lock funds.
The main impact that justifies the high severity is that, any existing user can perform this at any instant and hold the funds hostage, as soon as they discover the attack vector.
This also has a smaller impact of denying holder fees and protocol fees, as a result of denying any purchase itself.
Tools used
Manual review, hardhat for PoC
Recommended mitigation steps
Force the referral to claim fees by themselves, as opposed to transferring the fees to them on every transfer.
Important mitigation note: We highly recommend using a reentrancy guard if this mitigation method is used, as any external call, including ETH sending, can be dangerous.
The current contract setting does not employ any re-entrancy guard, and it currently does not follow the checks-effects-interaction pattern. If not done correctly, any ETH withdraw function may very well be the vector for draining the entire contract.
Assessed type
ETH-Transfer