search

code-423n4 / 2022-07-swivel-findings

0 stars 1 forks source link

Accidentally cancel order #121

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Swivel/Swivel.sol#L82-L104 https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Swivel/Swivel.sol#L244-L273

Vulnerability details

Accidentally cancel order

Impact

Some market maker's limit order might be accidentally cancelled, and has to be resubmitted again. If the user did not do this on purpose, without knowing the fact that the order has been cancelled, it may lead to unexpected disruption of market maker's plan.

Proof of Concept

I tried on the testnet, when the balance or allownance of DAI of the account is below the order amount, the system would automatically cancel the order in 2 minutes. However, sometimes the user might just spent the balance somewhere else, just forgets about the limit order requirement.

It might be better to transfer the amount of the fund as the order was placed. Since place and cancel incur the gas fee anyway. Making the transfer can eliminate the possiblity of accidentally cancel the order.

Tools Used

Mannual analysis.

Recommended Mitigation Steps

Transfer the fund required to the contract once the limit order has be placed.

JTraversa commented 2 years ago

The suggestion would break the UX of our product completely, and in general we've implemented the best practices for an off-chain orderbook.

bghughes commented 2 years ago

I agree with the Sponsor and feel that this is an invalid issue. General lack of clarity and it is unclear to me that is an attack vector of any kind. Moreover, the warden is referring to some of the off-chain order book logic which (given its off-chain nature) feels out of scope.