-
Notifications
You must be signed in to change notification settings - Fork 8
0xDetermination - LockingPositionDelegate: Addresses can be gas griefed and permanently DoSed such that they are unable to receive delegations #165
Comments
Escalate This issue is marked as a dup of #142, which I believe does not show sufficient impact. In my submission here, I demonstrate a gas griefing impact and a permanent DoS impact. The judge marked issue 142 as invalid with the reasoning 'Agree with sponsor, since cleanDelegatees() and removeTokenIdDelegated() here allow removal of delegatees'. This is true for issue 142, but it can be seen that neither of these two functions can be used to prevent the permanent DoS and gas griefing attacks described in my submission here. |
You've created a valid escalation! To remove the escalation from consideration: Delete your comment. You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final. |
This has the exact same root cause of the I will have to think more about the duplicates, given almost all have mentioned the correct root cause of abusing the max delegated cap. #142, #51, #49, #165 - I think this are definite duplicates, some forms of front running/back running mentioned. #212 - Seems to imply front-running, but debatable #206, #202 - Likely invalid, attack path described not sufficient. |
@nevillehuang Thank you very much for the detailed analysis. I took a look at these issues again and I agree with you on most of them. Have a few things I would like to mention, please let me know if my reasoning is good: #51 only describes the backrunning attack which is not 100% reliable- note that the potential failure of the backrunning attack is mentioned in this issue 165. (A backrunning attack can be circumvented with a flashbots bundle, or the backrunning tx simply might not be executed before another delegation tx). Similarly, #49 was submitted by the same watson, and it also does not describe a guaranteed DoS of over 1 year. I'm not sure if these two issues should be considered valid Medium/dup or valid low in this case, since DoS of under 1 year seems to be generally considered as low. #212 doesn't seem to imply frontrunning, and the recommendation is somewhat unclear and suggests that the watson was not aware of the cleaning/removal functions. I don't think this should be considered a Medium. Will respect your final decision. |
I continue to insist that the above reports describe different problems. Using a whitelist will not solve the problem for the token owner when one delegate needs to be de-delegated, although the intendent behavior of the There is no need for a special attack here. The function breaks down during normal user actions. This is exactly what is stated in Impact #206. So the owner of the token will delegate to trusted persons, he will be confident in their honesty and the absence of fraudulent activities, but as a result he will face the same problem. This will apply to all users without exception. I can’t comment on the validity of other items, because I don’t understand the benefit of an attacker in DOS of individual user addresses. And many questions arise: How does an attacker select addresses to attack? What will an attacker do if the delegate can accept delegation to different addresses? |
@ChechetkinVV I see your point, the different impact in #206 is that users will be unable to un-delegate once max delegations are reached (unless the delegatee cleans the list of delegated tokens). I agree that a whitelist would not fix the issue for this case. I also think you could make an argument for issue #206 to be considered separate from the rest of the delegation issues, but I don't think this specific impact can be considered H/M since it does not detail loss of funds or a DoS of over 1 year. Regarding benefit to the attacker- this is a griefing issue, and as a result there is no (obvious) benefit to the attacker. I think #142 implies a permanent DoS impact, so I won't argue for that issue to be judged low/invalid. I also don't think any of the other submissions demonstrate sufficient impact to be judged as H/M, as none of them describe DoS of over 1 year or gas griefing. |
I agree with @0xDetermination, the only attack vector here sufficient to make medium severity via a >1year or perhaps even permanent DoS is the possibility of front-running/backrunning a delegation removal from a delegator to invoke the max delegation check . If this isn't explicitly mentioned, seems to be hard to justify the severity on grounds of unclear attack path. |
I think griefing is not sufficient of an impact to be considered H/M according to Sherlock rules. Please note that frontrunning to get the transaction to fail is not a 1 year DoS, but a 1-2 block DoS. Doing that multiple times doesn't forbid the griefed user to bundle the transactions they want to make. The protocol will be on mainnet, so ways to bundle transactions exist (Flashbots, for example). Planning to leave the issue as is. |
@Czar102 , I didn't know bundling transactions using flashbots can be taken into considerations for such issues, will keep that in mind in the future. Best to make it clear to watsons in maybe sherlock rules, if not it can cause alot of unnecessary discussions in the future. |
@Czar102 @nevillehuang Thanks for the reply Czar102; I don't think tx bundling can fix this issue since the attacker can cause any |
I don't see how frontrunning causes a permanent DoS here. Please make sure you read my previous comment. |
@Czar102 Thanks very much for the reply. If I understand correctly, your point is that one frontrun will cause DoS of only one undelegating transaction, so the DoS impact is only 1 block and therefore does not constitute DoS of over 1 year. My argument is that the attacker can use a frontrunning bot to cause any undelegating transaction to fail, effectively causing permanent DoS. Example:
I think most of the marked duplicates of this issue only mentioned the backrunning attack vector, or otherwise failed to mention the guaranteed DoS vector via frontrunning; in these cases, transaction bundling would indeed circumvent the attack. Please kindly let me know if I am missing any key detail here. |
I understand the attack path, my point was that repetitively DoSing for a single transaction (griefing) is not considered a DoS for more than one year. Also, frontrunning doesn't change the crux of the attack – at this first (or second) stage, it is not important who undelegates the tokens, only provides a slightly more comfortable way of adding new tokens by the attacker. Hence, I don't think this observation provides a strong enough attack path to change the severity. |
@Czar102 Thanks very much for the reply, apologies for the invalid argument. |
Result: (duplication status doesn't matter) |
Escalations have been resolved successfully! Escalation status:
|
0xDetermination
medium
LockingPositionDelegate: Addresses can be gas griefed and permanently DoSed such that they are unable to receive delegations
Summary
Any address can be permanently DoSed such that it cannot receive delegations* (veCVG and/or mgCVG). Furthermore, gas griefing is possible.
*(technically, it still receives spam delegations from the attacker)
Vulnerability Detail
The explanation here will omit the case for mgCVG, as it's more or less the same as the case for veCVG.
Let's first consider the gas griefing impact. Consider the following code:
As can be seen, every address has a limit to how many delegations it can have. (This limit is set to 25 in the initializer.) Also note that
LockingPositionService.mintPosition()
does not have a minimum mint amount, so the cost in CVG to the attacker will be negligible. The attack is simple:removeTokenIdDelegated()
and remove at least one token from its delegator list. Note that delegators can only be removed one at a time.The gas cost to remove a delegator from a full list is at least 24 cold SLOADs (2100 gas each) due to the below code that's run when removing a delegator:
Therefore the gas cost is at least$24 * 2100 + 21000 = 71400$ , where 21000 is the base EVM transaction cost. The cost to the attacker for each delegating transaction (ignoring minting costs) is around 3 cold SLOADs (2100 gas each), 1 cold SSTORE (22100 for zero to nonzero), and 1 warm SSTORE (20000 for zero to nonzero), plus the base transaction cost. Thus the cost to the attacker to delegate one token is around 70,000 gas. It can be said that the cost/damage ratio is about 1:1.
Now, let's consider the permanent DoS attack. There are two possible attack scenarios. One attack uses frontrunning:
removeTokenIdDelegated()
for one of the attacker's tokens.delegateVeCVG()
foraddress(0)
, and the second delegates a new token to the victim.The other attack uses backrunning and is cheaper, but could potentially fail:
removeTokenIdDelegated()
for one of the attacker's tokens.The gas costs to the victim and the attacker in the backrunning scenario are about the same as in the gas griefing scenario; that is, 1:1.
In the frontrunning scenario, the attack is more expensive. The victim's transaction only uses 1 cold SLOAD before reverting, so the cost to the victim is around 25000 gas. In comparison, the attacker's first transaction executes 3 cold SLOADs and 1 warm SSTORE (2900 since the value is not changed to nonzero from zero), so the cost is around 30000 gas. We can assume that the second transaction costs around 70000 gas as previously explained, so the total cost to the attacker is around 100,000 and the ratio of the attacker cost to the victim cost for the guaranteed DoS attack is around 4:1.
Impact
Any address can be made permanently incapable of receiving delegations (DoS). If a victim address wants to receive delegations, it will need to spend gas to enable itself to do so (gas griefing, cost/damage ratio around 1:1).
Code Snippet
https://github.com/sherlock-audit/2023-11-convergence/blob/e894be3e36614a385cf409dc7e278d5b8f16d6f2/sherlock-cvg/contracts/Locking/LockingPositionDelegate.sol#L249
https://github.com/sherlock-audit/2023-11-convergence/blob/e894be3e36614a385cf409dc7e278d5b8f16d6f2/sherlock-cvg/contracts/Locking/LockingPositionDelegate.sol#L285
https://github.com/sherlock-audit/2023-11-convergence/blob/e894be3e36614a385cf409dc7e278d5b8f16d6f2/sherlock-cvg/contracts/Locking/LockingPositionService.sol#L238
https://github.com/sherlock-audit/2023-11-convergence/blob/e894be3e36614a385cf409dc7e278d5b8f16d6f2/sherlock-cvg/contracts/Locking/LockingPositionDelegate.sol#L66
https://github.com/sherlock-audit/2023-11-convergence/blob/e894be3e36614a385cf409dc7e278d5b8f16d6f2/sherlock-cvg/contracts/Locking/LockingPositionDelegate.sol#L248-L322
https://github.com/sherlock-audit/2023-11-convergence/blob/e894be3e36614a385cf409dc7e278d5b8f16d6f2/sherlock-cvg/contracts/Locking/LockingPositionDelegate.sol#L376-L383
https://github.com/sherlock-audit/2023-11-convergence/blob/e894be3e36614a385cf409dc7e278d5b8f16d6f2/sherlock-cvg/contracts/Locking/LockingPositionDelegate.sol#L422-L439
https://github.com/sherlock-audit/2023-11-convergence/blob/e894be3e36614a385cf409dc7e278d5b8f16d6f2/sherlock-cvg/contracts/Locking/LockingPositionDelegate.sol#L466-L483
Tool used
Manual Review
Recommendation
A whitelist could be implemented to prevent malicious actors from spam delegating.
If a whitelist is undesirable, the code architecture may require significant changes to fix this issue. If possible, keeping track of delegatee state should be done without storing lists of delegators.
The text was updated successfully, but these errors were encountered: