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

2 stars 1 forks source link

CreateOffererLib#createOrderHash function can be front-run by attacker and cause user create order failed #242

Closed c4-submissions closed 11 months ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/libraries/CreateOffererLib.sol#L274-L281 https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/DelegateToken.sol#L280-L283 https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/libraries/DelegateTokenStorageHelpers.sol#L117-L120

Vulnerability details

Impact

Function CreateOffererLib#calculateOrderHashAndId is used to calculate ERC20/ERC721/ERC1155 order hash and delegateTokenId. It create delegateTokenId parameter by calling DelegateTokenStorageHelpers#delegateIdNoRevert function, this function calculate delegateTokenId by uint256(keccak256(abi.encode(caller, salt))) sha256 hash algorithm, then call StorageHelpers#revertAlreadyExisted. This function will revert if delegateTokenId already existed because code below:

function revertAlreadyExisted(mapping(uint256 delegateTokenId => uint256[3] info) storage delegateTokenInfo, uint256 delegateTokenId) internal view {
    if (delegateTokenInfo[delegateTokenId][REGISTRY_HASH_POSITION] == ID_AVAILABLE) return;
    revert Errors.AlreadyExisted(delegateTokenId);
}

Proof of Concept

  1. An innocent user want to create an ERC721 order by calling CreateOfferer#calculateERC721OrderHashAndId function.
  2. Attacker monitor mempool and front-run the tx by calling CreateOfferer#calculateERC721OrderHashAndId with same parameters.
  3. The user create order failed because the delegateTokenId already existed.

Tools Used

vscode, manual review

Recommended Mitigation Steps

Considering use msg.sender to calculate delegateTokenId.

Assessed type

MEV

GalloDaSballo commented 12 months ago

This seems invalid since the new ID will be: -> PrevId keccak((keccak(original_user, salt), orderHash)

c4-sponsor commented 11 months ago

0xfoobar (sponsor) disputed

c4-judge commented 11 months ago

GalloDaSballo marked the issue as unsatisfactory: Insufficient proof