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

Absence of reserves length check during pump initialization can lead to storage corruption.

n0kto

Description

During an update, several checks are performed:

  • capReserves asserts that the length of the reserve is 2.

  • _getSlotsOffset, also called in _init, assert the length is not 0 (otherwise, it reverts due to underflow).

However, there is a path during the _init of a pump (if lastTimestamp is 0) that doesn't check if the reserve length exceeds 2. This could mislead a Well creator into thinking that the pump will work with more than 2 reserves, especially since many parts of the code are designed to handle more than 2 reserves. But any subsequent call with more than 2 reserves will revert.

Furthermore, if the reserve exceeds 3, it will write into the EMA reserve slots, causing the EMA last reserves to be incorrect until the next update.

function _init(
bytes32 slot,
uint40 lastTimestamp,
uint256[] memory reserves
) internal {
uint256 numberOfReserves = reserves.length;
bytes16[] memory byteReserves = new bytes16[](numberOfReserves);
​
// Skip {_capReserve} since we have no prior reference
​
for (uint256 i; i < numberOfReserves; ++i) {
uint256 _reserve = reserves[i];
if (_reserve == 0) return;
byteReserves[i] = _reserve.fromUIntToLog2();
}
​
// Write: Last Timestamp & Last Reserves
@> slot.storeLastReserves(lastTimestamp, reserves);
​
// Write: EMA Reserves
// Start at the slot after `byteReserves`
@> uint256 numSlots = _getSlotsOffset(numberOfReserves);
assembly {
@> slot := add(slot, numSlots)
}
@> slot.storeBytes16(byteReserves); // EMA Reserves
}

Risk

Likelyhood: Low

  • If a pump is initialized (accidentaly or intentionally) with more than 2 reserves.

Impact: High

  • Slot corruption leading to unexpected behavior for users of a Well.

  • Confusing for Well implementers.

Proof of Concept

Foundry PoC to add in `Pump.Update.t.sol`
function test_initWithNArray() public {
MultiFlowPump pump2 = new MultiFlowPump();
uint256[] memory reserves = new uint256[](3);
reserves[0] = 101;
reserves[1] = 102;
reserves[2] = 103;
​
vm.warp(250);
​
// won't revert!!
pump2.update(reserves, data);
}

Recommended Mitigation

Instead of using the condition below in _capReserves function, use it at the beginning of the update function:

if (reserves.length != 2) {
revert TooManyTokens();
}
Updates

Lead Judging Commences

giovannidisiena Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Reserves initialization

pontifex Judge
about 1 year ago
giovannidisiena Lead Judge
about 1 year ago
giovannidisiena Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Reserves initialization

Support

FAQs

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