dforce-network / dToken

dForce yield token
MIT License
31 stars 10 forks source link

Handler approvals may revert for non-standard tokens #25

Closed mgcolburn closed 4 years ago

mgcolburn commented 4 years ago

Severity: Informational

Description

In order to mitigate the well-known ERC20 race condition, some tokens disallow approvals being set to non-zero values when there is already an existing allowance set. Some well-known tokens that take this approach are USDT, ANT, SNT and any other token built from a MiniMeToken. Unfortunately, though this does help to address the race condition, this behavior violates the expected ERC20 behavior. As a result, calls to approve() in the *Handler contracts that interact with these tokens will succeed the first time they are called and fail on every subsequent call. This function does not take a parameter for the actual value to set the allowance to, and instead attempts to re-set the allowance to MAX_UINT256 on every call.

This issue is not likely to be practically exploitable, as the quantity of transfers required to exhaust an allowance this high is not practical for most tokens. However, it should still be fixed.

Recommendation

Modify the function parameters to take a specific allowance amount as a variable instead of using MAX_UINT256 universally.

joyious commented 4 years ago

Fixed in 188f4de