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

0 stars 1 forks source link

Wrong `allowance` logic #158

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Creator/ZcToken.sol#L132-L134 https://github.com/code-423n4/2022-07-swivel/blob/fd36ce96b46943026cb2dfcb76dfa3f884f51c18/Creator/ZcToken.sol#L111-L115

Vulnerability details

Impact

The logic around the decrementing the allowance in the withdraw and redeem methods of the contract ZcToken are wrong implemented and cannot be used.

Proof of Concept

There are a Denial of Service in the withdraw and redeem methods of the ZcToken contract, the allowance is checked in the opposite way, so if the allowed amount is greater or equal to the amount, it will be reverted, otherwise, an integer overflow will also revert the execution.

uint256 allowed = allowance[holder][msg.sender];
if (allowed >= previewAmount) {
    revert Approvals(allowed, previewAmount);
}
allowance[holder][msg.sender] -= previewAmount;

Recommended Mitigation Steps

   uint256 allowed = allowance[holder][msg.sender];
-  if (allowed >= previewAmount) {
+  if (allowed < previewAmount) {
       revert Approvals(allowed, previewAmount);
   }
   allowance[holder][msg.sender] -= previewAmount;
ghost commented 2 years ago

Duplicate of https://github.com/code-423n4/2022-07-swivel-findings/issues/180.

JTraversa commented 2 years ago

Duplicate of #129

robrobbins commented 2 years ago

addressed: https://github.com/Swivel-Finance/gost/pull/409

bghughes commented 2 years ago

Duplicate of #129