code-423n4 / 2023-09-delegate-findings

2 stars 1 forks source link

Unchecked Arithmetic Allows Nonce Replay #352

Closed c4-submissions closed 10 months ago

c4-submissions commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/libraries/CreateOffererLib.sol#L185

Vulnerability details

Vulnerability details

The problem is the unchecked increment operation: ++nonce.value;. When nonce.value is already at its maximum value (2^256 - 1), incrementing it will wrap around to zero due to integer overflow. This means that if an attacker sends a transaction with a contractNonce that matches the maximum nonce.value, the validation will pass, and the nonce will be reset to zero. As a result, the same nonce can be used again, leading to replay attacks or other problems.

Impact and Proof of Concept

an attacker can carry out replay attacks or manipulate the contract's state in unintended ways by reusing nonces. This can lead to financial losses or other security breaches, depending on the contract's functionality. let's say a scenario where nonce.value is at its maximum value (2^256 - 1), and a transaction attempts to increment it:

nonce.value = 2**256 - 1;  // Set nonce.value to its maximum value
processNonce(nonce, nonce.value + 1);  // Increment nonce.value without bounds check

In this case, nonce.value is incremented without checking for overflow, and it wraps around to zero, allowing the same nonce to be used again. This could potentially lead to unintended consequences depending on how the nonce is utilized in the contract's logic.

Tools Used

manual review

Recommended Mitigation Steps

using SafeMath libary to perform safe arithmetic operations and here is an updated version of the processNonce function using SafeMath as an example

import "@openzeppelin/contracts/utils/math/SafeMath.sol";

function processNonce(CreateOffererStructs.Nonce storage nonce, uint256 contractNonce) internal {
    if (nonce.value != contractNonce) revert CreateOffererErrors.InvalidContractNonce(nonce.value, contractNonce);
    nonce.value = SafeMath.add(nonce.value, 1);
}

Assessed type

Other

c4-judge commented 10 months ago

GalloDaSballo marked the issue as unsatisfactory: Invalid

GalloDaSballo commented 10 months ago

You'd need to sign, as a human being 2^256-1 txs which are more than the atoms in the universe