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

2 stars 1 forks source link

The tokenType is concatenated rather than tightly integrated. An attacker could manipulate just the type byte of the hash. #330

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#L393-L401 https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/libraries/CreateOffererLib.sol#L378-L383

Vulnerability details

Impact

The attacker can create an unintended type of order and asset transfer.

Proof of Concept

The tokenType is concatenated rather than tightly integrated. An attacker could manipulate just the type byte of the hash. The issue is that the tokenType is concatenated to the hash by shifting and OR'ing rather than tightly integrated via abi.encode. An attacker could manipulate just the last byte of the createOrderHash to change the tokenType. This would pass the validation in validateCreateOrderHash since only the full hash is compared. For example, the attacker could change an ERC721 order hash to an ERC1155 order hash by flipping the last bit of the hash from 0x01 to 0x03. This could allow the attacker to create an unintended type of order and asset transfer.

Tools Used

Manual

Recommended Mitigation Steps

The tokenType should be tightly integrated into the hash via abi.encode rather than concatenation

Assessed type

Other

c4-judge commented 11 months ago

GalloDaSballo marked the issue as unsatisfactory: Insufficient proof