Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

salt based offchain order contoller vulnerable to signature replay attacks

1stephen

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
over 1 year ago
inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Too generic

Appeal created

1stephen Submitter
over 1 year ago
inallhonesty Lead Judge
over 1 year ago
1stephen Submitter
over 1 year ago
inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Too generic

Support

FAQs

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

Give us feedback!