BriVault
First Flight #52
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: medium
Invalid

Lack of input validation for ERC20 transfers

eze001

Root + Impact

Description

  • Normally, token transfers to and from the vault should validate amounts and ensure that operations succeed, preventing unexpected failures or stuck funds.

  • The contract does not check return values of transferFrom or transfer, which can cause deposits or withdrawals to fail silently if the token does not return a boolean or behaves differently.

// Root cause in the codebase with @> marks to highlight the relevant section
pragma solidity ^0.8.0;
​
interface IERC20 {
function transferFrom(address from, address to, uint256 amount) external returns (bool);
function transfer(address to, uint256 amount) external returns (bool);
}
​
contract ERC20Validation {
IERC20 public token;
​
function deposit(uint256 amount) external {
@> token.transferFrom(msg.sender, address(this), amount); // return value unchecked
}
​
function withdraw(uint256 amount) external {
@> token.transfer(msg.sender, amount); // return value unchecked
}
}
​

Risk

Likelihood:

  • Occurs whenever the vault interacts with ERC20 tokens that do not follow the standard properly or fail during transfer.

  • Occurs whenever transfer or transferFrom return false, which the contract currently ignores.

Impact:

  • Impact 1: Deposits or withdrawals can fail silently, leaving user balances inconsistent.

  • Impact 2: Funds may become stuck in the vault or users may lose confidence due to failed operations.

Proof of Concept

The PoC shows that if an ERC20 token’s transfer or transferFrom fails or returns false, the vault does not detect it. This can cause deposits or withdrawals to fail silently, leaving user balances inconsistent or funds stuck in the vault.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
contract MaliciousToken {
mapping(address => uint256) public balanceOf;
​
function transferFrom(address, address, uint256) external pure returns (bool) {
return false; // always fails
}
​
function transfer(address, uint256) external pure returns (bool) {
return false; // always fails
}
}
​
contract ERC20ValidationPoC {
ERC20Validation public vault;
MaliciousToken public token;
​
constructor(ERC20Validation _vault, MaliciousToken _token) {
vault = _vault;
token = _token;
}
​
function attemptDeposit(uint256 amt) external {
vault.deposit(amt); // fails silently
}
​
function attemptWithdraw(uint256 amt) external {
vault.withdraw(amt); // fails silently
}
}
​

Recommended Mitigation

Always check the return value of transfer and transferFrom to ensure token operations succeed, preventing failed deposits or withdrawals and avoiding stuck funds.

- token.transferFrom(msg.sender, address(this), amount);
- token.transfer(msg.sender, amount);
+ require(token.transferFrom(msg.sender, address(this), amount), "transferFrom failed");
+ require(token.transfer(msg.sender, amount), "transfer failed");
​
Updates

Appeal created

bube Lead Judge 21 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!