DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: medium
Valid

Pricefeed's heartbeat is not set

Summary

The Chainlink price feed’s heartbeat for the token traded in a marketId is not configured and cannot be updated later.
This will lead to a massive DoS leaving protocol unusable.

Vulnerability Details

There are 2 priceFeedHeartbeatSeconds that must be configured.
First one is for the price feed for the collateral token. This is done by calling GlobalConfigurationBranch.configureMarginCollateral -> MarginCollateralConfiguration.configure()

//GlobalConfigurationBranch.sol
function configureMarginCollateral(
address collateralType,
uint128 depositCap,
uint120 loanToValue,
address priceFeed,
uint32 priceFeedHeartbeatSeconds
)
external
onlyOwner
{
try ERC20(collateralType).decimals() returns (uint8 decimals) {
if (decimals > Constants.SYSTEM_DECIMALS || priceFeed == address(0) || decimals == 0) {
revert Errors.InvalidMarginCollateralConfiguration(collateralType, decimals, priceFeed);
}
MarginCollateralConfiguration.configure(
collateralType, depositCap, loanToValue, decimals, priceFeed, priceFeedHeartbeatSeconds
);
//...
}
//MarginCollateralConfiguration.sol
function configure(
...
uint32 priceFeedHearbeatSeconds
)
internal
{
Data storage self = load(collateralType);
//...
// @audit collateral's pricefeed heartbeet is set; OK
self.priceFeedHearbeatSeconds = priceFeedHearbeatSeconds;
}

The second priceFeedHeartbeatSeconds is for price feed of the traded token. When a new perp market is created a priceFeedHeartbeatSeconds is passed down to PerpMarket.create then to MarketConfiguration's update -> self.configuration.update

If we check the update function the priceFeedHeartbeatSeconds is not saved in storage:

/// @notice Updates the given market configuration.
/// @dev See {MarketConfiguration.Data} for parameter details.
function update(Data storage self, Data memory params) internal {
self.name = params.name;
self.symbol = params.symbol;
self.priceAdapter = params.priceAdapter;
self.initialMarginRateX18 = params.initialMarginRateX18;
self.maintenanceMarginRateX18 = params.maintenanceMarginRateX18;
self.maxOpenInterest = params.maxOpenInterest;
self.maxSkew = params.maxSkew;
self.maxFundingVelocity = params.maxFundingVelocity;
self.minTradeSizeX18 = params.minTradeSizeX18;
self.skewScale = params.skewScale;
self.orderFees = params.orderFees;
//@audit priceFeedHeartbeatSeconds is not saved in configuration
}

Same update function is called when admin wants to update the market configuration:

function updatePerpMarketConfiguration(
uint128 marketId,
UpdatePerpMarketConfigurationParams calldata params
)
external
onlyOwner
onlyWhenPerpMarketIsInitialized(marketId)
{
PerpMarket.Data storage perpMarket = PerpMarket.load(marketId);
MarketConfiguration.Data storage perpMarketConfiguration = perpMarket.configuration;
//...
if (params.priceFeedHeartbeatSeconds == 0) {
revert Errors.ZeroInput("priceFeedHeartbeatSeconds");
}
//@audit same update function is called;
// priceFeedHeartbeatSeconds is not saved
perpMarketConfiguration.update(
MarketConfiguration.Data({
name: params.name,
symbol: params.symbol,
priceAdapter: params.priceAdapter,
initialMarginRateX18: params.initialMarginRateX18,
maintenanceMarginRateX18: params.maintenanceMarginRateX18,
maxOpenInterest: params.maxOpenInterest,
maxSkew: params.maxSkew,
maxFundingVelocity: params.maxFundingVelocity,
minTradeSizeX18: params.minTradeSizeX18,
skewScale: params.skewScale,
orderFees: params.orderFees,
priceFeedHeartbeatSeconds: params.priceFeedHeartbeatSeconds
})
);
emit LogUpdatePerpMarketConfiguration(msg.sender, marketId);
}

Heartbeat can't be updated after market creation meaning that its value will always be 0.

The non-initialize priceFeedHeartbeatSeconds is used in PerpMarket.getIndexPrice() -> ChainlinkUtil.getPrice

//PerpMarket.sol
function getIndexPrice(Data storage self) internal view returns (UD60x18 indexPrice) {
address priceAdapter = self.configuration.priceAdapter;
uint32 priceFeedHeartbeatSeconds = self.configuration.priceFeedHeartbeatSeconds;
GlobalConfiguration.Data storage globalConfiguration = GlobalConfiguration.load();
address sequencerUptimeFeed = globalConfiguration.sequencerUptimeFeedByChainId[block.chainid];
if (priceAdapter == address(0)) {
revert Errors.PriceAdapterNotDefined(self.id);
}
indexPrice = ChainlinkUtil.getPrice(
//@audit priceFeedHeartbeatSeconds is 0
IAggregatorV3(priceAdapter), priceFeedHeartbeatSeconds, IAggregatorV3(sequencerUptimeFeed)
);
}

Unless price was updated in same block when getPrice is queried, block.timestamp - updatedAt will always be bigger than 0, meaning that this function will revert.
Given than the heartbeat for BTC/USD feed is 24h or 0.05% price change, in periods with low volatility, the price will not be updated for up to 24h.

//ChainlinkUtil.sol
function getPrice(
IAggregatorV3 priceFeed,
uint32 priceFeedHeartbeatSeconds,
IAggregatorV3 sequencerUptimeFeed
)
internal
view
returns (UD60x18 price)
{
//...
try priceFeed.latestRoundData() returns (uint80, int256 answer, uint256, uint256 updatedAt, uint80) {
//@audit since priceFeedHeartbeatSeconds is 0 this will always revert
if (block.timestamp - updatedAt > priceFeedHeartbeatSeconds) {
revert Errors.OraclePriceFeedHeartbeat(address(priceFeed));
}
//...
} catch {
revert Errors.InvalidOracleReturn();
}
}

It's easy to test this by slightly addapting an existing test. In this case I modified testFuzz_GivenCallPerformUpkeepFunction test from test/integration/external/chainlink/keepers/liquidation/performUpkeep/performUpkeep.t.sol

Add the following test in file mentioned above. Execute it with forge test --mt test_CheckHeartBeat -vvv.

It will fail with the following error message: (key: "Error", value: "heartbeat from exposed permMarket != heartbeat from fuzzMarket")

function test_CheckHeartbeat()
external
givenInitializeContract
{
uint128 marketId = 1;
bool isLong = true;
uint256 amountOfTradingAccounts = 2;
MarketConfig memory fuzzMarketConfig = getFuzzMarketConfig(marketId);
amountOfTradingAccounts = bound({ x: amountOfTradingAccounts, min: 1, max: 10 });
uint256 marginValueUsd = 10_000e18 / amountOfTradingAccounts;
uint256 initialMarginRate = fuzzMarketConfig.imr;
deal({ token: address(usdz), to: users.naruto.account, give: marginValueUsd });
uint128[] memory accountsIds = new uint128[](amountOfTradingAccounts + 1);
uint256 accountMarginValueUsd = marginValueUsd / (amountOfTradingAccounts + 1);
for (uint256 i; i < amountOfTradingAccounts; i++) {
uint128 tradingAccountId = createAccountAndDeposit(accountMarginValueUsd, address(usdz));
openPosition(fuzzMarketConfig, tradingAccountId, initialMarginRate, accountMarginValueUsd, isLong);
accountsIds[i] = tradingAccountId;
}
setAccountsAsLiquidatable(fuzzMarketConfig, isLong);
uint128 nonLiquidatableTradingAccountId = createAccountAndDeposit(accountMarginValueUsd, address(usdz));
accountsIds[amountOfTradingAccounts] = nonLiquidatableTradingAccountId;
changePrank({ msgSender: users.owner.account });
LiquidationKeeper(liquidationKeeper).setForwarder(users.keepersForwarder.account);
changePrank({ msgSender: users.keepersForwarder.account });
bytes memory performData = abi.encode(accountsIds);
for (uint256 i; i < accountsIds.length; i++) {
if (accountsIds[i] == nonLiquidatableTradingAccountId) {
continue;
}
// it should emit a {LogLiquidateAccount} event
vm.expectEmit({
checkTopic1: true,
checkTopic2: true,
checkTopic3: false,
checkData: false,
emitter: address(perpsEngine)
});
emit LiquidationBranch.LogLiquidateAccount({
keeper: liquidationKeeper,
tradingAccountId: accountsIds[i],
amountOfOpenPositions: 0,
requiredMaintenanceMarginUsd: 0,
marginBalanceUsd: 0,
liquidatedCollateralUsd: 0,
liquidationFeeUsd: 0
});
}
//basically here the test starts
LiquidationKeeper(liquidationKeeper).performUpkeep(performData);
PerpMarket.Data memory perpMarket = PerpMarketHarness(perpsEngine).exposed_PerpMarket_load(uint128(marketId));
assertEq(perpMarket.configuration.priceFeedHeartbeatSeconds,
fuzzMarketConfig.priceFeedHeartbeatSeconds,
"heartbeat from exposed permMarket != heartbeat from fuzzMarket"
);
}

ChainlinkUtil.getPrice function doesn't revert because the updatedAtreturned by latestRoundData() is equal to block.timestamp, making the if check equivalent to if( x-x > 0) revert OraclePriceFeedHeartbeat()which is false.updatedAt

Impact

Protocol is unusable.

Tools Used

Manual review

Recommendations

Save the heartbeat inside MarketConfiguration `update()` :

//MarketConfiguration.sol
function update(Data storage self, Data memory params) internal {
self.name = params.name;
self.symbol = params.symbol;
self.priceAdapter = params.priceAdapter;
self.initialMarginRateX18 = params.initialMarginRateX18;
self.maintenanceMarginRateX18 = params.maintenanceMarginRateX18;
self.maxOpenInterest = params.maxOpenInterest;
self.maxSkew = params.maxSkew;
self.maxFundingVelocity = params.maxFundingVelocity;
self.minTradeSizeX18 = params.minTradeSizeX18;
self.skewScale = params.skewScale;
self.orderFees = params.orderFees;
+ self.priceFeedHeartbeatSeconds = params.priceFeedHeartbeatSeconds;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

`MarketConfiguration::update` function lacks `priceFeedHeartbeatSeconds` argument

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!