Beanstalk: The Finale
Beanstalk
DeFiHardhatFoundry
250,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Penalty Fees for Pipeline Convert Applied Incorrectly on L2

holydevoti0n

Summary

The LibConvert uses the block.number to prevent flash loan attacks that can cause price manipulation.

I asked the devs why they decided to use block.number instead of block.timestamp. Here's the reply from @pizzaman1337:

Using block number seemed cleaner than timestamp to me, since the idea was to limit converts on a per-block basis.

The problem with this approach is the following:

  1. Beanstalk determines the "conversion capacity" based on the capped reserves.

LibPipelineConvert reads the overall cappedDeltaB and passes it into the overallConvertCapacity -> prepareStalkPenaltyCalculation which passes it to LibConvert.applyStalkPenalty. Look at the snippet code below:

function executePipelineConvert(
address inputToken,
address outputToken,
uint256 fromAmount,
uint256 fromBdv,
uint256 initialGrownStalk,
AdvancedFarmCall[] calldata advancedFarmCalls
) external returns (uint256 toAmount, uint256 newGrownStalk, uint256 newBdv) {
...
// Store the capped overall deltaB, this limits the overall convert power for the block
@> pipeData.overallConvertCapacity = LibConvert.abs(LibDeltaB.overallCappedDeltaB());
...
​
pipeData.stalkPenaltyBdv = prepareStalkPenaltyCalculation(
inputToken,
outputToken,
pipeData.deltaB,
@> pipeData.overallConvertCapacity,
fromBdv,
pipeData.initialLpSupply
);
...
}
​
function prepareStalkPenaltyCalculation(
address inputToken,
address outputToken,
LibConvert.DeltaBStorage memory dbs,
uint256 overallConvertCapacity,
uint256 fromBdv,
uint256[] memory initialLpSupply
) public returns (uint256) {
...
return
LibConvert.applyStalkPenalty(
dbs,
fromBdv,
@> overallConvertCapacity,
inputToken,
outputToken
);
}
​
​

Finally, LibConvert -> applyStalkPenalty and LibConvert -> calculateConvertCapacityPenalty use this value to determine the overall convert capacity and the penalty fee:

function applyStalkPenalty(
DeltaBStorage memory dbs,
uint256 bdvConverted,
uint256 overallConvertCapacity,
address inputToken,
address outputToken
) internal returns (uint256 stalkPenaltyBdv) {
AppStorage storage s = LibAppStorage.diamondStorage();
uint256 overallConvertCapacityUsed;
uint256 inputTokenAmountUsed;
uint256 outputTokenAmountUsed;
​
​
(
stalkPenaltyBdv,
overallConvertCapacityUsed,
inputTokenAmountUsed,
outputTokenAmountUsed
) = calculateStalkPenalty(
dbs,
bdvConverted,
@> overallConvertCapacity,
inputToken,
outputToken
);
​
​
// Update penalties in storage.
@> ConvertCapacity storage convertCap = s.sys.convertCapacity[block.number];
@> convertCap.overallConvertCapacityUsed = convertCap.overallConvertCapacityUsed.add(
overallConvertCapacityUsed
);
@> convertCap.wellConvertCapacityUsed[inputToken] = convertCap
.wellConvertCapacityUsed[inputToken]
.add(inputTokenAmountUsed);
@> convertCap.wellConvertCapacityUsed[outputToken] = convertCap
.wellConvertCapacityUsed[outputToken]
.add(outputTokenAmountUsed);
}
​
function calculateConvertCapacityPenalty(
uint256 overallCappedDeltaB,
uint256 overallAmountInDirectionOfPeg,
address inputToken,
uint256 inputTokenAmountInDirectionOfPeg,
address outputToken,
uint256 outputTokenAmountInDirectionOfPeg
) internal view returns (uint256 cumulativePenalty, PenaltyData memory pdCapacity) {
AppStorage storage s = LibAppStorage.diamondStorage();
​
@> ConvertCapacity storage convertCap = s.sys.convertCapacity[block.number];
...
// update overall remaining convert capacity
@> pdCapacity.overall = convertCap.overallConvertCapacityUsed.add(
overallAmountInDirectionOfPeg
);
​
}

The readCappedReserves(MultiflowPump v1.1.0) works in the following manner:

  • Updates the reserves if the delta time is > 0.

  • Return the the last capped reserves if delta time == 0 or it caps the current reserves based on the data defined for the ConstantProduct2.

MultiflowPump -> readCappedReserves:

function readCappedReserves(
address well,
bytes calldata data
) external view returns (uint256[] memory cappedReserves) {
(, uint256 capInterval, CapReservesParameters memory crp) =
abi.decode(data, (bytes16, uint256, CapReservesParameters));
bytes32 slot = _getSlotForAddress(well);
uint256[] memory currentReserves = IWell(well).getReserves();
uint8 numberOfReserves;
uint40 lastTimestamp;
(numberOfReserves, lastTimestamp, cappedReserves) = slot.readLastReserves();
if (numberOfReserves == 0) {
revert NotInitialized();
}
@> uint256 deltaTimestamp = _getDeltaTimestamp(lastTimestamp);
@> if (deltaTimestamp == 0) {
@> return cappedReserves;
}
​
uint256 capExponent = calcCapExponent(deltaTimestamp, capInterval);
cappedReserves = _capReserves(well, cappedReserves, currentReserves, capExponent, crp);
}

Vulnerability Details

As shown above, Basin works with block. timestamp to return the capped reserves and avoid price manipulation in the same block. But Beanstalk is working with block.number. Due to this approach, Beanstalk is now exposed to the following issues when deployed on L2:

  • L2s batches transactions into single L1 blocks, thus multiple L2 blocks can share the same L1 block number. It means that some L2s like Arbitrum, the value of block.number can repeat for several blocks.

References:
Arbitrum

  • In general, the block.number on L2s is not a reliable source of timing information and the time between each block is also different from Ethereum. This is because each transaction on L2 is placed in a separate block and blocks are not produced at a constant rate.

References:
OZ audit report,
C4 Audit

Impact

  • Users may incur penalty fees for conversions due to executing their transactions in a block where the block.number has already reached its full capacity for conversions.

  • Users are vulnerable to being front-run by bots that exploit the full conversion capacity of a block. Bots can execute conversions first, using up the block’s capacity, and causing subsequent user transactions to incur penalty fees, thus burning their Grown Stalk unfairly.

Tools Used

Manual Review & Foundry

Recommendations

Replace block.number with block.timestamp to ensure accurate timing and prevent issues on L2s.

For Arbitrum, use ArbSys(100).arbBlockNumber() to obtain a unique Arbitrum-specific block number.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

Known - LightChaser

Support

FAQs

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