search

sherlock-audit / 2024-02-rubicon-finance-judging

5 stars 3 forks source link

pkqs90 - Orders with equal `decayStartTime` and `decayEndTime` benefit the filler instead of swapper #17

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 7 months ago

pkqs90

medium

Orders with equal decayStartTime and decayEndTime benefit the filler instead of swapper

Summary

GladiusReactor uses a linear decay function to adjust the input and output amounts within the specified time range between decayStartTime and decayEndTime. An edge case arises when decayStartTime and decayEndTime are set to the same value, but startAmount and endAmount differ. In such a scenario, the trade's final amount defaults to endAmount, which benefits the filler over the swapper.

Vulnerability Detail

As stated in the Summary.

This issue was actually fixed in the latest UniswapX in this PR https://github.com/Uniswap/UniswapX/pull/194, however it still exists in this codebase.

Impact

Swappers might be unaware of this specific edge case, potentially leading them to unintentionally place orders that favor the filler instead of benefiting themselves.

Code Snippet

DutchDecayLib.sol

    function decay(
        uint256 startAmount,
        uint256 endAmount,
        uint256 decayStartTime,
        uint256 decayEndTime
    ) internal view returns (uint256 decayedAmount) {
>       if (decayEndTime < decayStartTime) {
            revert EndTimeBeforeStartTime();
>       } else if (decayEndTime <= block.timestamp) {
            decayedAmount = endAmount;
        } else if (decayStartTime >= block.timestamp) {
            decayedAmount = startAmount;
        } else {
            ...

Tool used

VSCode

Recommendation

Disable dutch orders with zero duration to avoid ambiguity.

     ) internal view returns (uint256 decayedAmount) {
-        if (decayEndTime < decayStartTime) {
+        if (decayEndTime <= decayStartTime) {
             revert EndTimeBeforeStartTime();
         } else if (decayEndTime <= block.timestamp) {
sherlock-admin commented 7 months ago

2 comment(s) were left on this issue during the judging contest.

PNS commented:

L-03 OZ Audit; Dutch Orders Without Duration Benefit the Filler

0xAadi commented:

Invalid: OOS, contract not in scope

daoio commented 7 months ago

the library is out of scope