Lucene search
K
Database
Vendors
Products
Years
CVSS
Scanner
Agent Scanning
API Scanning
Manual Audit
Perimeter Scanner
Scanning
Projects
Email
Webhook
Plugins
Resources
Documents
Blog
Glossary
FAQ
Pricing
Contacts
About Us
Partners
Branding Guideline
code423n4Code4renaCODE423N4:2022-12-PREPO-FINDINGS-ISSUES-331
HistoryDec 12, 2022 - 12:00 a.m.

Unsafe usage of ERC20 methods

2022-12-1200:00:00
Code4rena
github.com
6
vulnerability
erc20
transfer errors
collateral
deposit
withdraw
deposithook
withdrawhook
deposittradehelper
JSON

Lines of code
<https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/Collateral.sol#L45-L61&gt;
<https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/Collateral.sol#L80-L83&gt;
<https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/DepositHook.sol#L43-L52&gt;
<https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/WithdrawHook.sol#L53-L79&gt;
<https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/DepositTradeHelper.sol#L22-L34&gt;

Vulnerability details

There are many weird ERC20 tokens that don’t follow the standard ERC20 interface.

Depending on the ERC20 token, some transfer errors may result in passing unnoticed, or some successful transfers may be treated as failed.

The current implementation assumes that these operations always revert on failure and always return a boolean value, which is not always the case.

Impact

Given the different usages of token transfers in the Collateral, DepositHook, WithdrawHook and DepositTradeHelper contracts there can be 2 types of issues depending on the ERC20 token being used.

One kind of issue is related to the usage of the boolean return value to signal success or failure. These tokens return a false value instead of reverting the operation. In this case, if the return value isn’t checked, a failed call may be taken as valid. For example, the deposit function in the Collateral contract uses IERC20.transferFrom to pull funds from the user (line 49). If the user doesn’t have the needed funds to deposit, the call will silently fail, and the function will still mint collateral tokens to the user representing a valid deposit.

Another type of impact is caused by tokens that don’t return a boolean value at all (a well known example of this type of token is USDT). Here, successful transfers will still revert the whole operation because Solidity is expecting to decode a return value and will fail. Following the example of the deposit function in the Collateral contract, the function call will still fail and revert even if the user has the proper funds.

Occurrences and code snippets

These are the occurrences of the ERC20 calls in the context of the arbitrary “base token” used for collateral:

PoC

Assuming a faulty ERC20 that signals success with the boolean return value (for example, the ZRX token):

function transfer(address _to, uint _value) returns (bool) {
    //Default assumes totalSupply can't be over max (2^256 - 1).
    if (balances[msg.sender] &gt;= _value && balances[_to] + _value &gt;= balances[_to]) {
        balances[msg.sender] -= _value;
        balances[_to] += _value;
        Transfer(msg.sender, _to, _value);
        return true;
    } else { return false; }
}

function transferFrom(address _from, address _to, uint _value) returns (bool) {
    if (balances[_from] &gt;= _value && allowed[_from][msg.sender] &gt;= _value && balances[_to] + _value &gt;= balances[_to]) {
        balances[_to] += _value;
        balances[_from] -= _value;
        allowed[_from][msg.sender] -= _value;
        Transfer(_from, _to, _value);
        return true;
    } else { return false; }
}

An attacker can execute the following steps to steal the funds in the Collateral contract:

  1. Use an account that doesn’t have any tokens (i.e. IERC20(baseToken).balanceOf(account) == 0).
  2. Call Collateral.deposit with any amount > 0. The transferFrom call will silently fail and the user will still be minted the collateral tokens.
  3. Call Collateral.withdraw with the minted amount to withdraw real funds from the contract.

Recommendation

Consider using OZ SafeERC20 (or similar) to handle ERC20 transferFrom, transfer and approve.

The text was updated successfully, but these errors were encountered:

All reactions

JSON