DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: low
Invalid

Lack of protocol version in validating the off-chain orders

Summary

During filling an order, the current version of the protocol is not verified to match the version of the protocol when the order was created.

Vulnerability Details

Suppose Alice creates an off-chain order. Before this order is filled, the protocol (or any branch) is upgraded. Then the Alice's off-chain order will be filled in the new version of the protocol. But, maybe it is not desirable for Alice because she is not happy with new configuration of the newly-upgraded protocol. She already signed the off-chain order based on the old configuration/structure of the protocol not the new one.

The root cause of this issue is that the version is not included during verification of off-chain order.

ctx.structHash = keccak256(
abi.encode(
Constants.CREATE_OFFCHAIN_ORDER_TYPEHASH,
ctx.offchainOrder.tradingAccountId,
ctx.offchainOrder.marketId,
ctx.offchainOrder.sizeDelta,
ctx.offchainOrder.targetPrice,
ctx.offchainOrder.shouldIncreaseNonce,
ctx.offchainOrder.nonce,
ctx.offchainOrder.salt
)
);

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/branches/SettlementBranch.sol#L240

The same scenario could be possible for on-chain orders, where the users creates a market order, but the protocol upgrades before this order is filled. It does not check that the order was created in older version of the protocol.

Impact

  • Lack of version verification during filling orders.

Tools Used

Recommendations

Following is recommended for off-chain orders (similar modification could be used for on-chain orders):

struct Data {
uint128 tradingAccountId;
uint128 marketId;
int128 sizeDelta;
uint128 targetPrice;
bool shouldIncreaseNonce;
uint120 nonce;
bytes32 salt;
uint8 v;
bytes32 r;
bytes32 s;
string version;
}

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/leaves/OffchainOrder.sol#L16

ctx.structHash = keccak256(
abi.encode(
Constants.CREATE_OFFCHAIN_ORDER_TYPEHASH,
ctx.offchainOrder.tradingAccountId,
ctx.offchainOrder.marketId,
ctx.offchainOrder.sizeDelta,
ctx.offchainOrder.targetPrice,
ctx.offchainOrder.shouldIncreaseNonce,
ctx.offchainOrder.nonce,
ctx.offchainOrder.salt,
ctx.version
)
);
require(keccack256(ctx.version) == keccack256(ZAROS_DOMAIN_VERSION), "the off-chain order is not compatible with the version of the protocol");

https://github.com/Cyfrin/2024-07-zaros/blob/main/src/perpetuals/branches/SettlementBranch.sol#L240

Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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