stake.link
stake.link
DeFiHardhatBridge
27,500 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

No safety check in `addToken`

larsson

Summary

SDLPool.addToken() lacks a safety mechanism to prevent the same token from being added multiple times.

Vulnerability Details

There's no safety check in SDLPool.addToken(). There's a possible exploit scenario that might happen:

One token being added twice in the rewards pool. The token would be counted doubly in the mapping(address => IRewardsPool) public tokenPools;.

File : contracts/core/sdlPool/base/SDLPool.sol
​
336: function addToken(address _token, address _rewardsPool) public override onlyOwner {
337: if (_token == address(sdlToken)) revert InvalidToken();
338: super.addToken(_token, _rewardsPool);
339: }

https://github.com/Cyfrin/2023-12-stake-link/blob/549b2b8c4a5b841686fceb9c311dca9ac58225df/contracts/core/sdlPool/base/SDLPool.sol#L336-L339

Proof of Concept

This scenario is illustrated in a minimalist example, which you can use in Remix:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.15;
​
contract SimpleTokenPool {
// Representing the token as an address for simplicity.
address[] public tokens;
​
// Only the owner (deployer for simplicity) can add tokens
address public owner;
​
constructor() {
owner = msg.sender;
}
​
// Modifier to restrict functions to the owner only.
modifier onlyOwner() {
require(msg.sender == owner, "Not owner");
_;
}
​
// Function to add a token to the pool.
function addToken(address _token) public onlyOwner {
// Notice there's no check to see if the token is already added.
tokens.push(_token);
}
​
// Function to check if a token is already in the pool (not used in addToken).
function isTokenAdded(address _token) public view returns (bool) {
for (uint i = 0; i < tokens.length; i++) {
if (tokens[i] == _token) {
return true;
}
}
return false;
}
}

Impact

Adding the same token twice would not raise any error here. I consider this a high-risk issue.

Tools Used

  • Manual code review

  • Remix

Recommendations

This behaviour can be mitigated by implementing the following require() statement:

+ mapping(address => bool) private _tokenRegistry;
...
function addToken(address _token, address _rewardsPool) public override onlyOwner {
+ require(!_tokenRegistry[_token], "Token already added");
+ _tokenRegistry[_token] = true;
if (_token == address(sdlToken)) revert InvalidToken();
super.addToken(_token, _rewardsPool);
}
Updates

Lead Judging Commences

0kage Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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