The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

The `LiquidationPool:claimRewards` function can be dos, If user is not able to receive ETH.

0xaman

Summary

Inside LiquidationPool::claimReward function users are able to claim their rewards. The rewards could be in ETH or other token supported by protocol, However the Contract does handle a case if user is not able to receive ETH. So this will lead to Dos on claiming the rewards. if such a case occur the users funds will remain stuck in protocol. likelihood on Low and severity in High thats why I have placed this finding in Medium.

Vulnerability Details

Code
function claimRewards() external {
ITokenManager.Token[] memory _tokens = ITokenManager(tokenManager).getAcceptedTokens();
for (uint256 i = 0; i < _tokens.length; i++) {
ITokenManager.Token memory _token = _tokens[i];
uint256 _rewardAmount = rewards[abi.encodePacked(msg.sender, _token.symbol)];
if (_rewardAmount > 0) {
delete rewards[abi.encodePacked(msg.sender, _token.symbol)];
if (_token.addr == address(0)) {
// @audit : this can be dosed in case of msg.sender is not able to receive ETH
(bool _sent,) = payable(msg.sender).call{value: _rewardAmount}("");
@> require(_sent);
} else {
IERC20(_token.addr).transfer(msg.sender, _rewardAmount);
}
}
​
}
}
​

impact

The claimRewards function check if user has rewards in ETH, it tries to send the rewards and check for its status , if status is false it will revert. but there is no other way to allow users to claim their rewards. even the token rewards will not be claimable due to it.

Tools Used

Manual Review

Recommendations

If ETH transfer fail try to send WETH instead of ETH. following changes will be required to handle this case.

@@ -9,6 +9,7 @@ import "contracts/interfaces/ILiquidationPool.sol";
import "contracts/interfaces/ILiquidationPoolManager.sol";
- import "contracts/interfaces/ISmartVaultManager.sol";
+ import "contracts/interfaces/ISmartVaultManagerV3.sol";
import "contracts/interfaces/ITokenManager.sol";
+import "contracts/interfaces/IWETH.sol";
​
@@ -162,74 +216,132 @@ contract LiquidationPool is ILiquidationPool {
}
function claimRewards() external {
+ ITokenManager.Token[] memory _tokens = ITokenManager(tokenManager)
+ .getAcceptedTokens();
for (uint256 i = 0; i < _tokens.length; i++) {
ITokenManager.Token memory _token = _tokens[i];
- uint256 _rewardAmount = rewards[abi.encodePacked(msg.sender, _token.symbol)];
if (_rewardAmount > 0) {
delete rewards[abi.encodePacked(msg.sender, _token.symbol)];
if (_token.addr == address(0)) {
+ (bool _sent, ) = payable(msg.sender).call{
+ value: _rewardAmount
+ }("");
+ if (!_sent) {
+ address weth = ISmartVaultManagerV3(manager).weth();
+ IWETH(address(weth)).deposit{value: _rewardAmount}();
+ // Transfer WETH instead
+ bool wethSuccess = IWETH(address(weth)).transfer(msg.sender, _rewardAmount);
+
+ // Ensure successful transfer
+ if (!wethSuccess) revert("WETH transfer failed");
+ }
} else {
​

also add implementation for WETHMock.sol as follows:

WETHMock.sol
​
// SPDX-License-Identifier: MIT
pragma solidity 0.8.17;
​
/// @title WETH
/// @notice FOR TEST PURPOSES ONLY.
contract WETHMock {
string public name = "Wrapped Ether";
string public symbol = "WETH";
uint8 public decimals = 18;
​
event Approval(address indexed src, address indexed guy, uint256 wad);
event Transfer(address indexed src, address indexed dst, uint256 wad);
event Deposit(address indexed dst, uint256 wad);
event Withdrawal(address indexed src, uint256 wad);
​
mapping(address => uint256) public balanceOf;
mapping(address => mapping(address => uint256)) public allowance;
​
fallback() external payable {
deposit();
}
​
receive() external payable {
deposit();
}
​
function deposit() public payable {
balanceOf[msg.sender] += msg.value;
emit Deposit(msg.sender, msg.value);
}
​
function withdraw(uint256 wad) public {
require(balanceOf[msg.sender] >= wad);
balanceOf[msg.sender] -= wad;
payable(msg.sender).transfer(wad);
emit Withdrawal(msg.sender, wad);
}
​
function totalSupply() public view returns (uint256) {
return address(this).balance;
}
​
function approve(address guy, uint256 wad) public returns (bool) {
allowance[msg.sender][guy] = wad;
emit Approval(msg.sender, guy, wad);
return true;
}
​
function transfer(address dst, uint256 wad) public returns (bool) {
return transferFrom(msg.sender, dst, wad);
}
​
function transferFrom(address src, address dst, uint256 wad) public returns (bool) {
require(balanceOf[src] >= wad);
​
if (src != msg.sender && allowance[src][msg.sender] != type(uint128).max) {
require(allowance[src][msg.sender] >= wad);
allowance[src][msg.sender] -= wad;
}
​
balanceOf[src] -= wad;
balanceOf[dst] += wad;
​
emit Transfer(src, dst, wad);
​
return true;
}
}
Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

informational/invalid

0xaman Submitter
over 1 year ago
hrishibhat Lead Judge
over 1 year ago
hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

informational/invalid

Support

FAQs

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