code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Tokens/ZcToken.sol#L111-L114 https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Tokens/ZcToken.sol#L132-L133

Vulnerability details

Impact / Description

In the ZcToken contract, the withdraw() and redeem() methods both support being called from an other account with an appropriate allowance set, but these functions fail to properly validate allowance.

The problem is the condition (allowed >= amount) to decide if the Approvals error must be raised. The condition should be the opposite, (allowed < amount).

These methods will always revert when called from an other account: the Approvals error will be raised when properly called, while solidity protection against overflow willhopefully protect the contract when called with an invalid allowance.

Recommended Mitigation Steps

The conditions to raise the Approvals error must be changed to use '<' instead of '>='.

Additional note

Allowance in the inherited contract Erc20 support using type(uint256).max as a special "permanent" allowance. The withdraw() and redeem() methods should also support this special value for better consistency.

JTraversa commented 2 years ago

Duplicate of #129

bghughes commented 2 years ago