c4-bot-7 commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibOrders.sol#L40-L54

Vulnerability details

Proof of Concept

Take a look at https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibOrders.sol#L40-L54

    function increaseSharesOnMatch(address asset, STypes.Order memory order, MTypes.Match memory matchTotal, uint88 eth) internal {
        AppStorage storage s = appStorage();

        // @dev use the diff to get more time (2159), to prevent overflow at year 2106
        uint32 timeTillMatch = getOffsetTime() - order.creationTime;
        if (timeTillMatch > C.MIN_DURATION) {
            // shares in eth-days
            uint88 shares = eth * (timeTillMatch / 1 days);
            matchTotal.dittoMatchedShares += shares;

            uint256 vault = s.asset[asset].vault;
            s.vaultUser[vault][order.addr].dittoMatchedShares += shares;
        }
    }

This is used as a helper function whenvever closing, case is that this directlyh deals with shares and while calculating the division is always ensured to come before the multiplication, i.e shares = eth * (timeTillMatch / 1 days).

Impact

Users unfairly have their shares truncated

Keep in mind that the division is by 1 days (86400) so this leads to quite a heavy truncation which is why this suffices as medium.

Recommended Mitigation Steps

Consider multiplying before dividing in increaseSharesOnMatch()

Assessed type

Token-Transfer

c4-pre-sort commented 3 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 3 months ago

raymondfam marked the issue as duplicate of #179

raymondfam commented 3 months ago

See #179.

c4-judge commented 3 months ago

hansfriese marked the issue as unsatisfactory: Out of scope

code-423n4 / 2024-03-dittoeth-findings