A malicious attacker can DoS an order by making its n and d invalid, without fully fulfilling the order.
Proof of Concept
In this example the attacker fulfills only 2/3 of the order, and makes the rest of it unfulfillable.
The attacker calls the fulfillAdvancedOder function with n = 1 and d = 4
The attacker calls the fulfillAdvancedOder function with n = 2 ** 116 and d = 2 ** 118
The function that calculates the fraction and the new n and d will cast the previous and the supplied n and d to uint256. Let's mark the previous n and d as fn and fd (filled numerator and filled denominator). Also let's mark the supplied n and d as sn and sd (supplied numerator and supplied denominator).
So fn, fd, sn, sd are all uint256s, and the new n will be sn * fd + sd * fn and the new d will be d * fd. With the values mentioned above, fn = 1, fd = 4, sn = 2 ** 116 and sd = 2 ** 118, which means that:
new fn = sn * fd + sd * fn = (2 ** 116) * (4) + (2 ** 118) * (1) = 2 ** 118 + 2 ** 118 = 2 ** 119new fd = d * fd = (4) * (2 ** 118) = 2 ** 120
But these are their values when they are uint256. When they will be casted back to uint120 in order to storage them back, their actual values will be:
new fn = (2 ** 119) % (2 ** 120) = 2 ** 119new fd = (2 ** 120) % (2 ** 120) = 0
So the new fraction will be invalid (because it won't be less than 1, i.e. n > d), and the next time someone will try to fulfill that order it'll revert.
Tools Used
Remix, VS Code and my brain.
Recommended Mitigation Steps
Check that n < type(uint120).max && d < type(uint120).max before down-casting n and d from uint256 to uint120. That will prevent the invalid values of n and d from being saved to the storage.
Lines of code
https://github.com/code-423n4/2022-05-opensea-seaport/blob/4140473b1f85d0df602548ad260b1739ddd734a5/contracts/lib/OrderValidator.sol#L197-L239
Vulnerability details
Impact
A malicious attacker can DoS an order by making its n and d invalid, without fully fulfilling the order.
Proof of Concept
In this example the attacker fulfills only 2/3 of the order, and makes the rest of it unfulfillable.
fulfillAdvancedOder
function withn = 1
andd = 4
fulfillAdvancedOder
function withn = 2 ** 116
andd = 2 ** 118
uint256
. Let's mark the previous n and d as fn and fd (filled numerator and filled denominator). Also let's mark the supplied n and d as sn and sd (supplied numerator and supplied denominator). Sofn, fd, sn, sd
are alluint256
s, and the new n will besn * fd + sd * fn
and the new d will bed * fd
. With the values mentioned above,fn = 1
,fd = 4
,sn = 2 ** 116
andsd = 2 ** 118
, which means that:new fn = sn * fd + sd * fn = (2 ** 116) * (4) + (2 ** 118) * (1) = 2 ** 118 + 2 ** 118 = 2 ** 119
new fd = d * fd = (4) * (2 ** 118) = 2 ** 120
But these are their values when they areuint256
. When they will be casted back touint120
in order to storage them back, their actual values will be:new fn = (2 ** 119) % (2 ** 120) = 2 ** 119
new fd = (2 ** 120) % (2 ** 120) = 0
So the new fraction will be invalid (because it won't be less than 1, i.e.n > d
), and the next time someone will try to fulfill that order it'll revert.Tools Used
Remix, VS Code and my brain.
Recommended Mitigation Steps
Check that
n < type(uint120).max && d < type(uint120).max
before down-casting n and d from uint256 to uint120. That will prevent the invalid values of n and d from being saved to the storage.