`receivableCycles` can still be `> maxCycles` in `_receiveDripsResult`

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-drips/blob/9fd776b50f4be23ca038b1d0426e63a69c7a511d/src/Drips.sol#L283

Vulnerability details

Description

receivableCycles can still be > maxCycles in _receiveDripsResult

Line: https://github.com/code-423n4/2023-01-drips/blob/9fd776b50f4be23ca038b1d0426e63a69c7a511d/src/Drips.sol#L283

if (toCycle - fromCycle > maxCycles) {
    receivableCycles = toCycle - fromCycle - maxCycles;
    toCycle -= receivableCycles;
}

In this case if toCycle - fromCycle > 2 * maxCycles, then receivableCycles > maxCycles which should not be the case.

The value of toCycle is correct (MAX: maxCycles away from fromCycle) since:

// toCycle = toCycle - receivableCycles
// toCycle = toCycle - (toCycle - fromCycle - maxCycles)
// toCycle = 0 + (fromCycle + maxCycles)

However, in _receiveDrips, emit ReceivedDrips is called with receivableCycles which will no longer be the true reflected value. Any system that depends on this emit will therefore be wrong.

See line: https://github.com/code-423n4/2023-01-drips/blob/9fd776b50f4be23ca038b1d0426e63a69c7a511d/src/Drips.sol#L256

Tools Used

Manual Audit

Recommended Mitigation Steps

Set receivableCycles = maxCycles when toCycle - fromCycle > maxCycles. Make sure to fix the toCycle update line as well. toCycle = fronCycle + receivableCycles

[xmxanuel]

Mhm, I don't really see the problem you mention.

Maybe there is a misunderstanding about the value of maxCycle.

Here are the cases in my opinion:

Case 1: maxCycle is bigger than range

from cycle = 50
toCycle = 100
maxCycle: 70

the if condition is not true.

fromCycle: 100
toCycle: 50
receivableCycles: 0

This seems correct. Everything until latest finished cycle will be received.

Case 2: maxCycle is smaller than range

fromCycle = 50
toCycle = 100
maxCycle: 20

In that case, we want to receive the range from 50 to 70 and receivableCycles after the call should be 30.

Then the if condition is true.

if (100 - 50 > 20) {
   receivableCycles = 100 - 50 - 20;
   toCycle =  100 - receivableCycles;
}

These are correct in my opinion.

[CodeSandwich]

[dispute validity] What Manuel said. receiveableCycles is /// @param receivableCycles The number of cycles which still can be received., these is the number of NOT received cycles.

[c4-sponsor]

CodeSandwich marked the issue as sponsor disputed

[GalloDaSballo]

Closing for lack of proof

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Insufficient proof