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

0 stars 1 forks source link

Incorrect condition will always fail withdrawal #65

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-swivel/blob/main/Creator/ZcToken.sol#L111

Vulnerability details

Impact

Due to an incorrect approval check, the if condition will always lead to transaction reversal when withdrawal is requested for a holder (who is not msg.sender). This can lead to user unable to withdraw funds

Proof of Concept

  1. Let us see the withdraw function
function withdraw(uint256 underlyingAmount, address receiver, address holder) external override returns (uint256 principalAmount){
        ...
        // Transfer logic
        // If holder is msg.sender, skip approval check
        if (holder == msg.sender) {
            ...
        }
        else {
            uint256 allowed = allowance[holder][msg.sender];
            if (allowed >= previewAmount) {
                revert Approvals(allowed, previewAmount);
            }
            allowance[holder][msg.sender] -= previewAmount;
            ...
        }
    }
  1. Assume User A was approved amount 5 by User B such that
allowance[B][A] = 5;
  1. User A calls the withdraw function with holder as B. Since holder is not msg.sender, this moves to else condition.
else {
            uint256 allowed = allowance[holder][msg.sender];
            if (allowed >= previewAmount) {
                revert Approvals(allowed, previewAmount);
            }
            allowance[holder][msg.sender] -= previewAmount;
            ...
        }
  1. In this case lets assume previewAmount is 4

  2. allowed is calculated as 5 as allowance[B][A] = 5;

  3. Ideally User A should be able to withdraw since his allowance is greater than withdrawal amount but due to below incorrect condition withdraw fails

if (allowed >= previewAmount)  // This becomes 5>=4 which is true hence revert
{
                revert Approvals(allowed, previewAmount);
            }

Other Occurences

The redeem function also suffers from same fate where approval is incorrectly checked and same recommendation needs to be applied

Recommended Mitigation Steps

if (allowed < previewAmount) {
                revert Approvals(allowed, previewAmount);
            }
JTraversa commented 2 years ago

Duplicate of #129

robrobbins commented 2 years ago

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

bghughes commented 2 years ago

Duplicate of #129