Users can withdraw in the same block (if the delay is zero)

[code423n4]

Lines of code

https://github.com/code-423n4/2022-03-prepo/blob/f63584133a0329781609e3f14c3004c1ca293e71/contracts/core/Collateral.sol#L144-L146
https://github.com/code-423n4/2022-03-prepo/blob/f63584133a0329781609e3f14c3004c1ca293e71/contracts/core/Collateral.sol#L128-L131

Vulnerability details

Impact

If the withdraw delay is 0 (which represent no expiration, because otherwise the withdrawal request will expire the moment it was created), the protection of withdrawing only in the next block ("Withdrawals must be requested in a prior block via initiateWithdrawal(uint256 amount). The number of blocks until a request expires is settable by the vault owner(). This is mainly for mitigating the feasibility of a flash loan attack.") doesn't work, because this check is in the _processDelayedWithdrawal function which is called only if the delay is not 0.

That bug opens the system to flashloan attacks, assuming that the withdraw delay is 0.

Tools Used

VS Code and Remix

Recommended Mitigation Steps

A possible fix will be to require the condition of the block numbers also if the withdraw delay is zero.

[ramenforbreakfast]

duplicate of #36

[gzeoneth]

Not an issue if the delay is specifically set to 0, but agree that it is inconsistant with the doc "Withdrawals must be requested in a prior block". Downgrading to Low/QA. Closing in favor of warden's QA report #40