Zaros Part 1

Summary

In the GlobalConfigurationBranch::configureMarginCollateral function, there is no check to prevent setting a zero value for the priceFeedHeartbeatSeconds parameter, despite the tests explicitly stating the value shouldn't be 0.

Additionally, the MarginCollateralConfiguration::configure function, which is called by configureMarginCollateral, also lacks this crucial check. This dual oversight could allow the configuration of a price feed with no minimum update frequency, potentially leading to the use of stale price data.

Vulnerability Details

And MarginCollateralConfiguration::configure doesn't check for zero heartbeat duration either:

Proof of Cocnept

• Modify one of the contract inside the script/margin-collaterals folder and change the XXX_PRICE_FEED_HEARBEAT_SECONDS value to 0

• Deploy the configurations of the Perpetual Engine by running script/02_ConfigurePerpsEngine.s.sol as the owner

• Observe that the transaction succeeds, setting a zero heartbeat duration

Proof of Code:

Insert the following test in the test/integration/perpetuals/global-configuration-branch/configureMarginCollateral/configureMarginCollateral.t.sol file:

Impact

A zero heartbeat duration could lead to the acceptance and use of stale price data indefinitely. This may result in:

• Incorrect valuation of collateral assets

• Potential creation of undercollateralized positions

• Failure to trigger necessary liquidations

• Exploitation of price discrepancies by malicious actors

While the function is only callable by the owner and typically used during deployment, and can be reverted with an upgrade, an accidental misconfiguration could have severe economic consequences for the protocol and its users.

Tools Used

Manual review

Recommendations

Change the error Errors::InvalidMarginCollateralConfiguration in src/utils/Errors.sol to add the priceFeedHeartbeatSeconds variable:

Explicitly check for zero in the validation of GlobalConfigurationBranch::configureMarginCollateral:

Note:

This error slipped through because the ConfigureMarginCollateral_Integration_Test::testFuzz_RevertWhen_PriceFeedHeartbeatSecondsIsZero test should use a price feed address different from 0. However, the price feed is set to address(0), causing the test to revert but for the wrong reason.