The validateCreateOrderHash function is vulnerable to an incorrect token type being provided by the caller

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

Invalid token types could be used with encoded order info, breaking expectations of the contract. An attacker could create an order hash using different parameters than what is actually encoded in the orderInfo. This could potentially allow the attacker to create an invalid order that still passes the hash comparison

Proof of Concept

The validateCreateOrderHash() function takes in the targetTokenReceiver, createOrderHash, encodedOrder and tokenType as parameters. It then calculates the actualCreateOrderHash using the provided inputs: https://github.com/code-423n4/2023-09-delegate/blob/a6dbac8068760ee4fc5bababb57e3fe79e5eeb2e/src/libraries/CreateOffererLib.sol#L378-L383 The calculateOrderHash() encodes the order info and other parameters into a hash, and appends the tokenType as the last byte. If an attacker calls validateCreateOrderHash() with an encodedOrder of one token type, but provides a different tokenType parameter, the actualCreateOrderHash will be calculated incorrectly with the wrong token type appended. However, the comparison against the provided createOrderHash could still pass, since the attacker controls the createOrderHash value. This allows the attacker to bypass the hash comparison with an invalid tokenType.

In essence Here is how an attacker could exploit this:

1. The attacker calls validateCreateOrderHash() and provides: • targetTokenReceiver address • createOrderHash (generated by the attacker) • encodedOrderInfo for an ERC721 order • But specifies the tokenType as ERC20
2. Inside validateCreateOrderHash(), the actualCreateOrderHash is calculated using the encodedOrderInfo and tokenType provided by the attacker.
3. Even though the encodedOrderInfo is for an ERC721 order, the actualCreateOrderHash will use ERC20 as the tokenType.
4. This actualCreateOrderHash could potentially match the createOrderHash provided by the attacker.
5. So the comparison would pass even though the tokenType does not actually match the encodedOrderInfo.

Tools Used

Manual

Recommended Mitigation Steps

the calculateOrderHash() function should be changed to encode the tokenType directly in the hash, rather than appending it at the end.

Assessed type

Other

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid