Relevant GitHub Links
Refrences
Summary
The _depositViaBroker function within the contract allows for a broker.fee value of zero, which can lead to issues when interacting with ERC20 tokens that do not support zero-value transfers such as LEND. When brokerFeeAmount is calculated as zero and an attempt is made to transfer this zero amount to the broker’s account, the transaction will revert for tokens that do not support zero-value transfers.
Vulnerability Details:
The _depositViaBroker method accepts a Broker struct with an address and a fee. During execution, if broker.fee is set to zero, the brokerFeeAmount will also be zero. For tokens that revert on zero transfers, such as certain ERC20 implementations, this will cause the _streams[streamId].token.safeTransferFrom call to revert, interrupting the entire transaction.
function _depositViaBroker(uint256 streamId, uint128 totalAmount, Broker memory broker) internal {
(uint128 brokerFeeAmount, uint128 depositAmount) =
Helpers.checkAndCalculateBrokerFee(totalAmount, broker, MAX_FEE);
_deposit(streamId, depositAmount);
_streams[streamId].token.safeTransferFrom({ from: msg.sender, to: broker.account, value: brokerFeeAmount });
}
Impact Details
Allowing zero transfers can lead to reverts when working with certain ERC20 tokens that do not support this operation. If broker.fee is zero, the entire transaction could fail for these tokens, which may disrupt functionality and hinder integrations with tokens having this limitation. Users and integrators interacting with the _depositViaBroker function might experience unexpected failures if they use affected ERC20 tokens.
POC:
Add the following test file to the test folder:
tests\Chista0xAudit_2.t.sol
pragma solidity >=0.8.22;
import { Integration_Test } from "./integration/integration.t.sol";
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import { UD60x18 } from "@prb/math/src/UD60x18.sol";
import { console } from "forge-std/src/console.sol";
contract Chista0xAudit_2 is Integration_Test {
ERC20RevertZeroAmount internal lend;
UD60x18 internal constant ZERO_FEE = UD60x18.wrap(0.0e18);
uint128 internal constant TOTAL_AMOUNT_WITH_BROKER_ZERO_FEE_18D = 50_000e18;
function setUp() public override {
Integration_Test.setUp();
lend = new ERC20RevertZeroAmount("EthLend Token", "LEND", 18);
vm.label(address(lend), "LEND");
deal({ token: address(lend), to: users.sender, give: 1_000_000e18 });
lend.approve({ spender: address(flow), value: UINT256_MAX });
}
function test_WhenBrokerFeeZero()
external
whenNoDelegateCall
givenNotNull
givenNotVoided
whenSenderMatches
whenRecipientMatches
{
uint256 streamId = createDefaultStream(IERC20(address(lend)));
defaultBroker.fee = ZERO_FEE;
flow.depositViaBroker(
streamId, TOTAL_AMOUNT_WITH_BROKER_ZERO_FEE_18D, users.sender, users.recipient, defaultBroker
);
}
}
contract ERC20RevertZeroAmount is ERC20 {
uint8 internal immutable DECIMAL;
mapping(address => uint256) balances;
constructor(string memory name_, string memory symbol_, uint8 decimals_) ERC20(name_, symbol_) {
DECIMAL = decimals_;
}
function decimals() public view override returns (uint8) {
return DECIMAL;
}
function transferFrom(address _from, address _to, uint256 _value) public override returns (bool) {
require(balances[_to] + _value > balances[_to], "Zero Amount Transfer");
return super.transferFrom(_from, _to, _value);
}
function transfer(address _to, uint256 _value) public override returns (bool) {
require(balances[_to] + _value > balances[_to], "Zero Amount Transfer");
return super.transfer(_to, _value);
}
}
Run the test with the command forge test --mc Chista0xAudit_2
Test output:
Ran 1 test for tests/Chista0xAudit_2.t.sol:Chista0xAudit_2
[FAIL. Reason: revert: Zero Amount Transfer] test_WhenBrokerFeeZero() (gas: 216362)
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 33.26ms (467.50µs CPU time)
Ran 1 test suite in 35.58ms (33.26ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
Failing tests:
Encountered 1 failing test in tests/Chista0xAudit_2.t.sol:Chista0xAudit_2
[FAIL. Reason: revert: Zero Amount Transfer] test_WhenBrokerFeeZero() (gas: 216362)
Encountered a total of 1 failing tests, 0 tests succeeded
Recommendation:
Before executing the safeTransferFrom function, add a check to ensure brokerFeeAmount is greater than zero. This will prevent zero-value transfers, avoiding reverts for tokens that do not support this operation and improving compatibility with such tokens.
Tools Used
Manual code review / Foundry tests
