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

salt based offchain order contoller vulnerable to signature replay attacks

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

summary -The current implementation of the OffchainOrder library is vulnerable to signature replay attacks due to insufficient handling of salts (random 32-byte values) used to distinguish offchain orders. This vulnerability allows an attacker to create a second valid signature for a previously used order, enabling the order to be executed multiple times.

Vulnerability details- The OffchainOrder library relies on salts to ensure the uniqueness of each offchain order. However, in ECDSA signatures, each signature has a second valid s value (with a corresponding flipped v value) that can recover the same address. This means an attacker can produce a new, unique signature based on a previously used one without knowing the signer’s private key. Consequently, this vulnerability allows every order to be replayed once.

IMPACT- It allows an attacker to replay an offchain order, and can double the intended transaction.

tools used- manal review

recommendation- we can track used salts , check if they have been used, and after they have been used mark them as used .

proof of concept-

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.25;
library OffchainOrder {
struct Data {
uint128 tradingAccountId;
uint128 marketId;
int128 sizeDelta;
uint128 targetPrice;
bool shouldIncreaseNonce;
uint120 nonce;
bytes32 salt;
uint8 v;
bytes32 r;
bytes32 s;
uint256 expiration;
}
}
contract TradingPlatform {
using OffchainOrder for OffchainOrder.Data;
mapping(uint128 => mapping(bytes32 => bool)) private usedSalts;
mapping(uint128 => uint120) public nonces;
function executeOrder(OffchainOrder.Data memory order) external {
require(block.timestamp < order.expiration, "Order expired");
bytes32 messageHash = getMessageHash(order);
address signer = ecrecover(messageHash, order.v, order.r, order.s);
require(signer != address(0), "Invalid signature");
require(!usedSalts[order.tradingAccountId][order.salt], "Salt already used");
usedSalts[order.tradingAccountId][order.salt] = true;
require(order.nonce == nonces[order.tradingAccountId], "Invalid nonce");
nonces[order.tradingAccountId]++;
_processOrder(order);
}
function getMessageHash(OffchainOrder.Data memory order) internal pure returns (bytes32) {
return keccak256(abi.encodePacked(
order.tradingAccountId,
order.marketId,
order.sizeDelta,
order.targetPrice,
order.shouldIncreaseNonce,
order.nonce,
order.salt,
order.expiration
));
}
function _processOrder(OffchainOrder.Data memory order) internal {
// Order processing logic
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
12 months ago
inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Too generic

Appeal created

1stephen Submitter
11 months ago
inallhonesty Lead Judge
11 months ago
1stephen Submitter
11 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Too generic

Support

FAQs

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