Beanstalk: Dive Into Basin
Beanstalk
DeFiHardhat
12,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

`MultiFlowPump` would store the wrong pump reserves if `update()` is called twice in the same transaction.

bladesec

Summary

MultiFlowPump would store the wrong pump reserves if update() is called twice in the same transaction.

Vulnerability Details

updates() doesn't update the pump reserves if they have been updated already in the same transaction.

function update(uint256[] calldata reserves, bytes calldata data) external {
(bytes16 alpha, uint256 capInterval, CapReservesParameters memory crp) =
abi.decode(data, (bytes16, uint256, CapReservesParameters));
uint256 numberOfReserves = reserves.length;
PumpState memory pumpState;
​
// All reserves are stored starting at the msg.sender address slot in storage.
bytes32 slot = _getSlotForAddress(msg.sender);
​
// Read: Last Timestamp & Last Reserves
(, pumpState.lastTimestamp, pumpState.lastReserves) = slot.readLastReserves();
​
// If the last timestamp is 0, then the pump has never been used before.
if (pumpState.lastTimestamp == 0) {
_init(slot, uint40(block.timestamp), reserves);
return;
}
​
bytes16 alphaN;
bytes16 deltaTimestampBytes;
uint256 capExponent;
// Isolate in brackets to prevent stack too deep errors
{
uint256 deltaTimestamp = _getDeltaTimestamp(pumpState.lastTimestamp);
// If no time has passed, don't update the pump reserves.
if (deltaTimestamp == 0) return; //@audit
alphaN = alpha.powu(deltaTimestamp);
deltaTimestampBytes = deltaTimestamp.fromUInt();
// Round up in case capInterval > block time to guarantee capExponent > 0 if time has passed since the last update.
capExponent = calcCapExponent(deltaTimestamp, capInterval);
}
...
}

So if update() is called again with the changed Well reserves, it will just skip the changes and the pump reserves will store the old data.

uint256 _reserve;
for (uint256 i; i < numberOfReserves; ++i) {
// Use a minimum of 1 for reserve. Geometric means will be set to 0 if a reserve is 0.
_reserve = reserves[i];
pumpState.lastReserves[i] =
_capReserve(pumpState.lastReserves[i], (_reserve > 0 ? _reserve : 1).fromUIntToLog2(), capExponent);
pumpState.emaReserves[i] =
pumpState.lastReserves[i].mul((ABDKMathQuad.ONE.sub(alphaN))).add(pumpState.emaReserves[i].mul(alphaN));
pumpState.cumulativeReserves[i] =
pumpState.cumulativeReserves[i].add(pumpState.lastReserves[i].mul(deltaTimestampBytes));
}

So, in the next transaction, lastReserves will be calculated with the wrong saved reserve and the current reserve.

Impact

The pump reserves might be tracked wrongly in some cases.

Tools Used

Manual Review

Recommendations

The update() function should track the pump reserves with the last Well's reserves every time.

Updates

Lead Judging Commences

giovannidisiena Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Informational/Invalid

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.