sherlock-audit / 2023-06-dinari-judging

5 stars 4 forks source link

auditsea - `requestCancel` can cause DoS attack #112

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

auditsea

high

requestCancel can cause DoS attack

Summary

requestCancel function can cause DoS(or DDoS) attack

Vulnerability Detail

In OrderProcessor.sol, requestCancel function can be called as many times as possible while an order is active. So the attacker can create an malicious smart contract that creates an order and loop requesting cancellation of that order right after creation. Since there can be a delay between cancellation request event and actual cancellation, there can be literally as many cancellation request events as possible. This will cause off-chain operating service to handle all those events, which will consume off-chain resources and tons of gas fees for calling cancelOrder function even though it fails from 2nd time. Moreover, if attack is done from multiple different wallets with different orders(DDoS), it will be very hard for the service to blacklist each one.

Impact

This is a critical issue because based on the attack rate, the off-chain service will stop working while they process malicious event(which can be too many), also losing resources and uncountable gas fees. For attackers, they will consume very little gas fees for calling requestCancel compared to operators calling cancelOrder.

Code Snippet

https://github.com/sherlock-audit/2023-06-dinari/blob/main/sbt-contracts/src/issuer/OrderProcessor.sol#L313-L323

Tool used

Manual Review

Recommendation

Add a status to OrderState so that it can reject cancellations if it's already requested once.

Shogoki commented 1 year ago

Escalate This is not a valid duplicate of #57

sherlock-admin commented 1 year ago

Escalate This is not a valid duplicate of #57

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.

Oot2k commented 1 year ago

Agree, and think this is a low issue.

hrishibhat commented 1 year ago

Result: Invalid Has duplicates This can be considered a non-issue as it is not smart contract related

sherlock-admin2 commented 1 year ago

Escalations have been resolved successfully!

Escalation status:

ctf-sec commented 1 year ago

@jaketimothy

https://github.com/dinaricrypto/sbt-contracts/issues/113

the fix stores the state to make sure a order cannot be cancel multiple times