code-423n4 / 2021-09-swivel-findings

0 stars 0 forks source link

Missing input validation & event in emergency blockWithdrawal could be risky #107

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

0xRajeev

Vulnerability details

Impact

blockWithdrawal does not perform input validation to check if input address is non-zero, indeed has a non-zero withdrawals time as scheduled earlier via scheduleWithdrawal or if it is being executed within/outside the HOLD period.

Without such validation, the block could be accidentally attempted for an incorrect address, performed without regard for HOLD period (the override should ideally be allowed only with the HOLD timelock period), and cannot be monitored off-chain given the absence of event emission. Given that this is an emergency function, lack of such input validation and event emission could critically delay/affect incident response.

The missing event for such a critical onlyAdmin override function (along with missing event for the withdaw() function) which unconditionally overrides previously scheduled onlyAdmin withdrawals is by itself a concern.

Proof of Concept

https://github.com/Swivel-Finance/gost/blob/5fb7ad62f1f3a962c7bf5348560fe88de0618bae/test/swivel/Swivel.sol#L380-L384

Tools Used

Manual Analysis

Recommended Mitigation Steps

Add missing input validation on zero-address, non-zero value of withdrawals[e] and HOLD period consideration before resetting it to zero.

JTraversa commented 3 years ago

Duplicate of an event ticket for admin functionality: setFee / blockWithdrawal

JTraversa commented 2 years ago

https://github.com/Swivel-Finance/gost/pull/174/commits/e20e1804c9d3eac94d6c02651956cc12a7da4a26