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

Missing Zero Address Check in setForwarder Function

ayoashy

Summary

The BaseKeeper::setForwarder function does not include a zero address check for the forwarder parameter. This oversight allows the forwarder address to be set to a zero address, which can lead to potential security issues and disruptions in the intended functionality of the contract.

Vulnerability Details

The setForwarder function is intended to update the forwarder address in the BaseKeeperStorage. However, the function does not validate that the forwarder address is not a zero address before assigning it to storage.

Here is the relevant code snippet:

/// @notice Updates the Keeper forwarder address.
/// @param forwarder The new forwarder address.
function setForwarder(address forwarder) external onlyOwner {
// No zero address check on the forwarder address
BaseKeeperStorage storage self = _getBaseKeeperStorage();
self.forwarder = forwarder;
}
​
function _getBaseKeeperStorage() internal pure returns (BaseKeeperStorage storage self) {
bytes32 slot = BASE_KEEPER_LOCATION;
​
assembly {
self.slot := slot
}
}
​
struct BaseKeeperStorage {
address forwarder;
}
​

The setForwarder function does not perform a check to ensure the forwarder address is valid and not a zero address:

self.forwarder = forwarder;

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/external/chainlink/keepers/BaseKeeper.sol#L39-L42

Impact

Setting a zero address as the forwarder can cause disruptions in the contract's forwarding mechanism, potentially leading to failed transactions or loss of functionality.

Tools Used

Manual Review

Recommendations

To prevent this issue, a zero address check should be added to the setForwarder function to ensure that the forwarder address is always valid.

Here is the updated function with the necessary check:

/// @notice Updates the Keeper forwarder address.
/// @param forwarder The new forwarder address.
function setForwarder(address forwarder) external onlyOwner {
// Check for zero address forwarder
if (forwarder == address(0)) {
revert Errors.ZeroInput("forwarder");
}
​
BaseKeeperStorage storage self = _getBaseKeeperStorage();
self.forwarder = forwarder;
}
​
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.