else branch of _claim(), the return value and the Event parameter claimedAmount should not be 0.function claimAndStake(address _token, uint8 _percentage, Exchange _exchange, bytes calldata _data)
external
onlyAfterDate(startClaimDate)
{
uint256 claimedAmount = _claim(_token, address(this), _percentage, _exchange, _data);
lpETH.approve(address(lpETHVault), claimedAmount);
lpETHVault.stake(claimedAmount, msg.sender);
emit StakedVault(msg.sender, claimedAmount);
}
The returned claimedAmount will always be 0; as a result, no lpETH will be staked.
function _claim(address _token, address _receiver, uint8 _percentage, Exchange _exchange, bytes calldata _data)
internal
returns (uint256 claimedAmount)
{
uint256 userStake = balances[msg.sender][_token];
if (userStake == 0) {
revert NothingToClaim();
}
if (_token == ETH) {
claimedAmount = userStake.mulDiv(totalLpETH, totalSupply);
balances[msg.sender][_token] = 0;
lpETH.safeTransfer(_receiver, claimedAmount);
} else {
uint256 userClaim = userStake * _percentage / 100;
_validateData(_token, userClaim, _exchange, _data);
balances[msg.sender][_token] = userStake - userClaim;
// At this point there should not be any ETH in the contract
// Swap token to ETH
_fillQuote(IERC20(_token), userClaim, _data);
// Convert swapped ETH to lpETH (1 to 1 conversion)
lpETH.deposit{value: address(this).balance}(_receiver);
}
emit Claimed(msg.sender, _token, claimedAmount);
}
function _claim(address _token, address _receiver, uint8 _percentage, Exchange _exchange, bytes calldata _data)
internal
returns (uint256 claimedAmount)
{
uint256 userStake = balances[msg.sender][_token];
if (userStake == 0) {
revert NothingToClaim();
}
if (_token == ETH) {
claimedAmount = userStake.mulDiv(totalLpETH, totalSupply);
balances[msg.sender][_token] = 0;
lpETH.safeTransfer(_receiver, claimedAmount);
} else {
uint256 userClaim = userStake * _percentage / 100;
_validateData(_token, userClaim, _exchange, _data);
balances[msg.sender][_token] = userStake - userClaim;
// At this point there should not be any ETH in the contract
// Swap token to ETH
_fillQuote(IERC20(_token), userClaim, _data);
// Convert swapped ETH to lpETH (1 to 1 conversion)
claimedAmount = address(this).balance;
lpETH.deposit{value: claimedAmount}(_receiver);
}
emit Claimed(msg.sender, _token, claimedAmount);
}
withdraw() function reverts in an obscure manner or with an unclear reason message when emergencyMode && _token == ETH && block.timestamp >= startClaimDate due to the absence of an ETH.safeTransfer() function selector.This leads to an unintended execution reaching L297.
/**
* @dev Called by a staker to withdraw all their ETH or LRT
* Note Can only be called after the loop address is set and before claiming lpETH,
* i.e. for at least TIMELOCK. In emergency mode can be called at any time.
* @param _token Address of the token to withdraw
*/
function withdraw(address _token) external {
if (!emergencyMode) {
if (block.timestamp <= loopActivation) {
revert CurrentlyNotPossible();
}
if (block.timestamp >= startClaimDate) {
revert NoLongerPossible();
}
}
uint256 lockedAmount = balances[msg.sender][_token];
balances[msg.sender][_token] = 0;
if (lockedAmount == 0) {
revert CannotWithdrawZero();
}
if (_token == ETH && block.timestamp < startClaimDate) {
// If block.timestamp >= startClaimDate use claim to get your lpETH instead
totalSupply = totalSupply - lockedAmount;
(bool sent,) = msg.sender.call{value: lockedAmount}("");
if (!sent) {
revert FailedToSendEther();
}
} else {
IERC20(_token).safeTransfer(msg.sender, lockedAmount);
}
emit Withdrawn(msg.sender, _token, lockedAmount);
}
Consider changing to something like:
/**
* @dev Called by a staker to withdraw all their ETH or LRT
* Note Can only be called after the loop address is set and before claiming lpETH,
* i.e. for at least TIMELOCK. In emergency mode can be called at any time.
* @param _token Address of the token to withdraw
*/
function withdraw(address _token) external {
if (!emergencyMode) {
if (block.timestamp <= loopActivation) {
revert CurrentlyNotPossible();
}
if (block.timestamp >= startClaimDate) {
revert NoLongerPossible();
}
}
uint256 lockedAmount = balances[msg.sender][_token];
balances[msg.sender][_token] = 0;
if (lockedAmount == 0) {
revert CannotWithdrawZero();
}
if (_token == ETH) {
// If block.timestamp >= startClaimDate use claim to get your lpETH instead
require(block.timestamp < startClaimDate, "use claim to get your lpETH instead");
totalSupply = totalSupply - lockedAmount;
(bool sent,) = msg.sender.call{value: lockedAmount}("");
if (!sent) {
revert FailedToSendEther();
}
} else {
IERC20(_token).safeTransfer(msg.sender, lockedAmount);
}
emit Withdrawn(msg.sender, _token, lockedAmount);
}
_fillQuote() allowing outputToken == address(WETH) without converting to NATIVE token through WETH.withdraw()If it is expected that the _fillQuote() swap output token will always be a NATIVE token, it is recommended to more explicitly validate the outputToken in the calldata.
Consider changing from:
@@ 393,399 @@
/**
* @notice Validates the data sent from 0x API to match desired behaviour
* @param _token address of the token to sell
* @param _amount amount of token to sell
* @param _exchange exchange identifier where the swap takes place
* @param _data swap data from 0x API
*/
function _validateData(address _token, uint256 _amount, Exchange _exchange, bytes calldata _data) internal view {
@@ 401,405 @@
address inputToken;
address outputToken;
uint256 inputTokenAmount;
address recipient;
bytes4 selector;
if (_exchange == Exchange.UniswapV3) {
(inputToken, outputToken, inputTokenAmount, recipient, selector) = _decodeUniswapV3Data(_data);
if (selector != UNI_SELECTOR) {
revert WrongSelector(selector);
}
} else if (_exchange == Exchange.TransformERC20) {
(inputToken, outputToken, inputTokenAmount, selector) = _decodeTransformERC20Data(_data);
if (selector != TRANSFORM_SELECTOR) {
revert WrongSelector(selector);
}
} else {
revert WrongExchange();
}
if (inputToken != _token || (outputToken != ETH && outputToken != address(WETH))) {
revert WrongDataTokens(inputToken, outputToken);
}
@@ 424,429 @@
if (inputTokenAmount != _amount) {
revert WrongDataAmount(inputTokenAmount);
}
if (recipient != address(this) && recipient != address(0)) {
revert WrongRecipient(recipient);
}
}
To:
@@ 393,399 @@
/**
* @notice Validates the data sent from 0x API to match desired behaviour
* @param _token address of the token to sell
* @param _amount amount of token to sell
* @param _exchange exchange identifier where the swap takes place
* @param _data swap data from 0x API
*/
function _validateData(address _token, uint256 _amount, Exchange _exchange, bytes calldata _data) internal view {
@@ 401,405 @@
address inputToken;
address outputToken;
uint256 inputTokenAmount;
address recipient;
bytes4 selector;
if (_exchange == Exchange.UniswapV3) {
(inputToken, outputToken, inputTokenAmount, recipient, selector) = _decodeUniswapV3Data(_data);
if (selector != UNI_SELECTOR) {
revert WrongSelector(selector);
}
// UniswapV3Feature.sellTokenForEthToUniswapV3(encodedPath, sellAmount, minBuyAmount, recipient) requires `encodedPath` to be a Uniswap-encoded path, where the last token is WETH, and sends the NATIVE token to `recipient`
if (outputToken != address(WETH)) {
revert WrongDataTokens(inputToken, outputToken);
}
} else if (_exchange == Exchange.TransformERC20) {
(inputToken, outputToken, inputTokenAmount, selector) = _decodeTransformERC20Data(_data);
if (selector != TRANSFORM_SELECTOR) {
revert WrongSelector(selector);
}
if (outputToken != ETH) {
revert WrongDataTokens(inputToken, outputToken);
}
} else {
revert WrongExchange();
}
if (inputToken != _token) {
revert WrongDataTokens(inputToken, outputToken);
}
@@ 431,436 @@
if (inputTokenAmount != _amount) {
revert WrongDataAmount(inputTokenAmount);
}
if (recipient != address(this) && recipient != address(0)) {
revert WrongRecipient(recipient);
}
}