Summary
The GlobalConfigurationBranch::createPerpMarket function has incomplete input validation for critical components of the params argument. Specifically, it lacks checks for the keeper address in SettlementConfiguration.Data and the makerFee and takerFee in OrderFees.Data. This oversight means
A zero address could be set as the keeper in the SettlementConfiguration, potentially breaking keeper-dependent functionalities.
Zero Input values could be passed as takerFee and makerFee, risking economic imbalances in the market.
Vulnerability Details
1.) Missing Zero Address Check: The keeper address in SettlementConfiguration.Data is not checked for a zero address. This check is absent in GlobalConfigurationBranch::createPerpMarket function, PerpMarket::create function and the SettlementConfiguration::update function.
https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/branches/GlobalConfigurationBranch.sol#L379-L441
https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/leaves/PerpMarket.sol#L395-L429
https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/leaves/SettlementConfiguration.sol#L109-L123
function update(
uint128 marketId,
uint128 settlementConfigurationId,
Data memory settlementConfiguration
) internal {
self.keeper = settlementConfiguration.keeper;
}
2.) Incomplete Validation for OrderFees: The orderFees parameter, which contains makerFee and takerFee, is not validated for zero values in the createPerpMarket function.
function createPerpMarket(CreatePerpMarketParams calldata params) external onlyOwner {
if (params.marketId == 0) {
revert Errors.ZeroInput("marketId");
}
if (abi.encodePacked(params.name).length == 0) {
revert Errors.ZeroInput("name");
}
if (abi.encodePacked(params.symbol).length == 0) {
revert Errors.ZeroInput("symbol");
}
if (params.priceAdapter == address(0)) {
revert Errors.ZeroInput("priceAdapter");
}
if (params.maintenanceMarginRateX18 == 0) {
revert Errors.ZeroInput("maintenanceMarginRateX18");
}
if (params.maxOpenInterest == 0) {
revert Errors.ZeroInput("maxOpenInterest");
}
if (params.maxSkew == 0) {
revert Errors.ZeroInput("maxSkew");
}
if (params.initialMarginRateX18 <= params.maintenanceMarginRateX18) {
revert Errors.InitialMarginRateLessOrEqualThanMaintenanceMarginRate();
}
if (params.skewScale == 0) {
revert Errors.ZeroInput("skewScale");
}
if (params.minTradeSizeX18 == 0) {
revert Errors.ZeroInput("minTradeSizeX18");
}
if (params.maxFundingVelocity == 0) {
revert Errors.ZeroInput("maxFundingVelocity");
}
if (params.priceFeedHeartbeatSeconds == 0) {
revert Errors.ZeroInput("priceFeedHeartbeatSeconds");
}
}
Impact
Tools Used
Manual Review
Recommendations
1). Add Zero Address Check for Keeper: Ensure that the keeper address in SettlementConfiguration.Data is checked to confirm it is not a zero address by adding this line of code below:
if (params.marketOrderConfiguration.keeper == address(0)) {
revert Errors.ZeroInput("keeper");
}
if (params.offchainOrdersConfiguration.keeper == address(0)) {
revert Errors.ZeroInput("keeper");
}
2). Validate Order Fees: Include checks to ensure makerFee and takerFee are greater than zero by adding this line of code below:
if (params.orderFees.makerFee == 0) {
revert Errors.ZeroInput("makerFee");
}
if (params.orderFees.takerFee == 0) {
revert Errors.ZeroInput("takerFee");
}
the GlobalConfigurationBranch::createPerpMarket could be updated to:
function createPerpMarket(CreatePerpMarketParams calldata params) external onlyOwner {
if (params.marketId == 0) {
revert Errors.ZeroInput("marketId");
}
if (abi.encodePacked(params.name).length == 0) {
revert Errors.ZeroInput("name");
}
if (abi.encodePacked(params.symbol).length == 0) {
revert Errors.ZeroInput("symbol");
}
if (params.priceAdapter == address(0)) {
revert Errors.ZeroInput("priceAdapter");
}
if (params.maintenanceMarginRateX18 == 0) {
revert Errors.ZeroInput("maintenanceMarginRateX18");
}
if (params.maxOpenInterest == 0) {
revert Errors.ZeroInput("maxOpenInterest");
}
if (params.maxSkew == 0) {
revert Errors.ZeroInput("maxSkew");
}
if (params.initialMarginRateX18 <= params.maintenanceMarginRateX18) {
revert Errors.InitialMarginRateLessOrEqualThanMaintenanceMarginRate();
}
if (params.skewScale == 0) {
revert Errors.ZeroInput("skewScale");
}
if (params.minTradeSizeX18 == 0) {
revert Errors.ZeroInput("minTradeSizeX18");
}
if (params.maxFundingVelocity == 0) {
revert Errors.ZeroInput("maxFundingVelocity");
}
if (params.priceFeedHeartbeatSeconds == 0) {
revert Errors.ZeroInput("priceFeedHeartbeatSeconds");
}
if (params.marketOrderConfiguration.keeper == address(0)) {
revert Errors.ZeroInput("keeper");
}
if (params.offchainOrdersConfiguration.keeper == address(0)) {
revert Errors.ZeroInput("keeper");
}
if (params.orderFees.makerFee == 0) {
revert Errors.ZeroInput("makerFee");
}
if (params.orderFees.takerFee == 0) {
revert Errors.ZeroInput("takerFee");
}
}
