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

2 stars 1 forks source link

There is a potential vulnerability with the nonce not incrementing as expected if there is an error or revert during the seaport flow #322

Closed c4-submissions closed 11 months ago

c4-submissions commented 11 months ago

Lines of code

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

Vulnerability details

Impact

This would allow the attacker to reuse the same nonce in multiple seaport transactions by reverting the first transaction after processNonce increments the stored nonce.

Proof of Concept

There is a potential issue with the nonce not incrementing as expected if there is an error or revert during the seaport flow

  1. The nonce value is stored in a struct: https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/libraries/CreateOffererLib.sol#L72
  2. The nonce is validated and incremented in processNonce(): https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/libraries/CreateOffererLib.sol#L184-L189
  3. This processNonce function is called at the start of each stage of the seaport flow: generateOrder, transfer, ratify.

The potential vulnerability is that if any of those stages revert/throw an error, the nonce will not be incremented as expected.

For example:

Nonce starts at 0 generateOrder called, nonce validated as 0 and incremented to 1. generateOrder reverts for some reason nonce is still 1 attacker calls generateOrder again with nonce 1, will pass validation even though generateOrder was never fully executed This could allow reusing the same nonce and nonce-related attacks.

Tools Used

Manual

Recommended Mitigation Steps

The nonce increment should happen at the start of the function before any potential revert:

Assessed type

Other

c4-judge commented 11 months ago

GalloDaSballo marked the issue as unsatisfactory: Invalid