Submission Details
Severity:
Missing check for address(0) in constructor of Fees contract lead to permanently loose profits
Summary
Contract Fees does no check for initializing immutable variables to address(0) in constructor. Setting staking to address(0) is possible and when someone calls sellProfits function the amount is transferred as WETH doesn’t revert on transferring to address(0).
Vulnerability Details
Contract Fees
contract Fees {
address public immutable WETH;
address public immutable staking;
.
.
constructor(address _weth, address _staking) {
// no address(0) check
WETH = _weth;
staking = _staking; // can set to address(0)
}
/// @notice swap loan tokens for collateral tokens from liquidations
/// @param _profits the token to swap for WETH
function sellProfits(address _profits) public {
require(_profits != WETH, "not allowed");
uint256 amount = IERC20(_profits).balanceOf(address(this));
// uniswap logic
// weth.transfer(address(0), amount) -> success
IERC20(WETH).transfer(staking, IERC20(WETH).balanceOf(address(this)));
}
}
PoC
// File: tests/Fees.t.sol
// SPDX-License-Identifier: UNLICENSED
import "forge-std/Test.sol";
import "../src/Fees.sol";
import {WETH} from "solady/src/tokens/WETH.sol";
contract TWETH is Test {
WETH weth;
function setUp() public {
weth = new WETH();
weth.deposit{value: 10 * 1e18}();
}
function testWeth() public {
assertEq(weth.balanceOf(address(this)), 10*1e18);
}
// test pass -> weth doesn't revert on transfer to address(0)
function testTransferToAddressZero() public {
/*
src/Fees.sol::L43 IERC20(WETH).transfer(staking, IERC20(WETH).balanceOf(address(this)));
*/
weth.transfer(address(0), 1 * 1e18);
}
}
[PASS] testTransferToAddressZero() (gas: 34714)
Traces:
[34714] TWETH::testTransferToAddressZero()
├─ [29516] WETH::transfer(0x0000000000000000000000000000000000000000, 1000000000000000000 [1e18])
│ ├─ emit Transfer(from: TWETH: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], to: 0x0000000000000000000000000000000000000000, value: 1000000000000000000 [1e18])
│ └─ ← true
└─ ← ()
shows WETH does transferred amount to address(0).
Impact
User may loose funds.
Tools Used
Manual Code Review, Foundry
Recommendations
Add zero-address checks
constructor(address _weth, address _staking) {
+ require(_staking != address(0), "staking address can't be address(0)");
WETH = _weth;
staking = _staking;
}
Prize pool breakdown
Total prize
20,000 USDC
nSLOC
70628 USDC / LOC
18,000 USDC
2,000 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.