The Standard

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

ERC-20 symbols are not unique

Summary

The rewards mapping in the LiquidationPool contract uses a rewards mapping to track the rewards a user can claim after assets are distributed among all stakers. The key of the mapping is the concatenation of the user address and the ERC-20 symbol. This could lead to bad consequences if two ERC-20 tokens have the same symbol, as they would be treated as the same token.

Vulnerability Details

Here we can see how rewards are saved in the mapping:

rewards[abi.encodePacked(_position.holder, asset.token.symbol)] += _portion;

And here we can see how they are claimed:

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}("");
require(_sent);
} else {
IERC20(_token.addr).transfer(msg.sender, _rewardAmount);
}
}
}
}

A loop over all accepted tokens is performed, and the rewards are claimed for each token.

ERC-20 symbols are not unique, so it is possible that two ERC-20 tokens have the same symbol. If this happens, the rewards of both tokens will be saved in the same mapping key. This can lead to loss for the user, or the protocol, depending on which token appears first in the list of accepted tokens and if they both vary in value.

There are more functions like for example removeCollateral inside the SmartVaultV3 contract which rely on the token symbol:

function removeCollateral(bytes32 _symbol, uint256 _amount, address _to) external onlyOwner {
ITokenManager.Token memory token = getTokenManager().getToken(_symbol);
require(canRemoveCollateral(token, _amount), UNDER_COLL);
IERC20(token.addr).safeTransfer(_to, _amount);
emit CollateralRemoved(_symbol, _amount, _to);
}

Impact

If two ERC-20 tokens have the same symbol, the rewards of both tokens will be saved in the same mapping key. This can lead to loss for the user, or the protocol, depending on which token appears first in the list of accepted tokens and if they both vary in value. Other functionality in the contracts would also break when two tokens with the same symbol are used.

Recommendations

Use the token address instead of the symbol to save the rewards in the mapping.

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Out of scope
Assigned finding tags:

informational/invalid

Support

FAQs

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