The initializeGlobalData function can be called multiple times by any user, leading to unintended reinitialization of critical contract state variables. This can disrupt the contract's functioning by resetting important counters and configurations, which could be exploited by malicious actors to manipulate the contract state.
For example, reinitializing the global data could reset the pair count and vault count, causing conflicts and inconsistencies in subsequent operations that depend on these values.
Proof of Concept
This function can be called by any user without any restrictions, as shown below:
Deploy the contract and call initializeGlobalData.
Any user can call initializeGlobalData again, resetting the values of pairsCount and vaultCount.
Example:
Assuming the contract is deployed at address 0xContractAddress:
User A calls initializeGlobalData(0xGlobalData, 0xUniswapFactory).
User B calls initializeGlobalData(0xGlobalData, 0xAnotherUniswapFactory).
Each call resets pairsCount and vaultCount, demonstrating the lack of restriction and the potential for disruption.
Tools Used
Manual Review
Recommended Mitigation Steps
Add Access Control:
Use an onlyOwner modifier to restrict access to the function.
Ensure Single Initialization:
Add a check to prevent the function from being called more than once.
Lines of code
https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/libraries/logic/AddPairLogic.sol#L44-L48
Vulnerability details
Impact
The initializeGlobalData function can be called multiple times by any user, leading to unintended reinitialization of critical contract state variables. This can disrupt the contract's functioning by resetting important counters and configurations, which could be exploited by malicious actors to manipulate the contract state. For example, reinitializing the global data could reset the pair count and vault count, causing conflicts and inconsistencies in subsequent operations that depend on these values.
Proof of Concept
This function can be called by any user without any restrictions, as shown below:
Deploy the contract and call initializeGlobalData. Any user can call initializeGlobalData again, resetting the values of pairsCount and vaultCount. Example:
Assuming the contract is deployed at address 0xContractAddress:
User A calls initializeGlobalData(0xGlobalData, 0xUniswapFactory). User B calls initializeGlobalData(0xGlobalData, 0xAnotherUniswapFactory). Each call resets pairsCount and vaultCount, demonstrating the lack of restriction and the potential for disruption.
Tools Used
Manual Review
Recommended Mitigation Steps
Add Access Control: Use an onlyOwner modifier to restrict access to the function.
Ensure Single Initialization: Add a check to prevent the function from being called more than once.
function initializeGlobalData(GlobalDataLibrary.GlobalData storage global, address uniswapFactory) external onlyOwner { require(global.pairsCount == 0 && global.vaultCount == 0, "Already initialized"); global.pairsCount = 1; global.vaultCount = 1; global.uniswapFactory = uniswapFactory; }
Example with OpenZeppelin's Ownable:
import "@openzeppelin/contracts/access/Ownable.sol";
contract YourContract is Ownable { // ... other code ...
}
Assessed type
Other