Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Missing Zero Address Check in LiquidationKeeper::initialize Function

ayoashy

Summary

The LiquidationKeeper::initialize function lacks a zero address check for the owner argument. While the function correctly checks that perpsEngine is not a zero address, it omits this critical validation for the owner argument. This oversight could lead to unintended behavior and potential security vulnerabilities within the system.

Vulnerability Details

The initialize function is designed to set up the LiquidationKeeper contract with an owner and a perpsEngine. However, it does not verify that the owner address is not a zero address, which can lead to various issues if an invalid owner is set during initialization.

/// @notice {LiquidationKeeper} UUPS initializer.
function initialize(address owner, IPerpsEngine perpsEngine) external initializer {
__BaseKeeper_init(owner);
​
if (address(perpsEngine) == address(0)) {
revert Errors.ZeroInput("perpsEngine");
}
​
LiquidationKeeperStorage storage self = _getLiquidationKeeperStorage();
self.perpsEngine = perpsEngine;
}

The function includes a zero address check for perpsEngine but lacks a similar check for owner

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/external/chainlink/keepers/liquidation/LiquidationKeeper.sol#L25-L35

Impact

If the owner is set to a zero address, the contract may lose critical administrative functions, leading to potential security risks and loss of control over key operations.

Tools Used

Manual Review

Recommendations

To mitigate this vulnerability, a zero address check for the owner argument should be added to the initialize function. This ensures that the owner is always set to a valid address.

Here is the updated function with the necessary check:

/// @notice {LiquidationKeeper} UUPS initializer.
function initialize(address owner, IPerpsEngine perpsEngine) external initializer {
// Check for zero address owner
if (owner == address(0)) {
revert Errors.ZeroInput("owner");
}
​
__BaseKeeper_init(owner);
​
if (address(perpsEngine) == address(0)) {
revert Errors.ZeroInput("perpsEngine");
}
​
LiquidationKeeperStorage storage self = _getLiquidationKeeperStorage();
self.perpsEngine = perpsEngine;
}
​
Updates

Lead Judging Commences

inallhonesty 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.