delegate ID could differ from the expected order hash if the order hash was manipulated

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

A malicious user could create an order hash that does not match the actual order data. When the delegate token is created, the actualDelegateId will be different than the requestedDelegateId calculated from the manipulated createOrderHash. But the check will still pass since it is comparing against the manipulated hash.

Proof of Concept

The key parts of the code are:

1. createAndValidateDelegateTokenId(): This creates the delegate token and validates that the returned ID matches the expected ID calculated from the order hash.
2. calculateOrderHash(): This calculates the order hash from the order parameters. The vulnerability is that createAndValidateDelegateTokenId() assumes the order hash passed in was calculated correctly and matches the actual order parameters. But a malicious caller could manipulate the order hash to generate a specific delegate token ID they want, while passing different order parameters.

This could allow the attacker to get control of a delegate token ID that doesn't match the actual order.

In essence: The key functions are createAndValidateDelegateTokenId and calculateOrderHash. createAndValidateDelegateTokenId creates the delegate token using create and gets back an actualDelegateId. It then recalculates the expected delegate ID based on the createOrderHash using delegateIdNoRevert. If these two IDs don't match, it reverts. The vulnerability is that createOrderHash could be manipulated before calling createAndValidateDelegateTokenId. A malicious user could create an order hash that does not match the actual order data. When the delegate token is created, the actualDelegateId will be different than the requestedDelegateId calculated from the manipulated createOrderHash. But the check will still pass since it is comparing against the manipulated hash.

A proof of concept attack:

• Attacker calls calculateOrderHash() with specific parameters to generate orderHash1.
• Attacker passes orderHash1 into createAndValidateDelegateTokenId() but specifies different order parameters.
• The delegate token is created with ID matching orderHash1, even though the actual parameters differ.
• Attacker now controls the delegate token ID from orderHash1 with different order rights.

Tools Used

Manual

Recommended Mitigation Steps

createAndValidateDelegateTokenId() should recalculate the expected order hash from the passed in order parameters, and compare it to the passed in orderHash before creating the delegate token.

Assessed type

Other

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid