DeFiHardhatFoundry
250,000 USDC
View results
Submission Details
Severity: high
Invalid

``ship()`` function of LibShipping breaks the ``cap`` invariant.

Summary

ship() function of LibShipping breaks the cap invariant.

Vulnerability Details

There is a cap amount set for every plan in ShipmentPlanner.sol. ship() function of LibShipping has no access control thus anyone can call this function again and again with any amount of beansToShip.

function ship(uint256 beansToShip) public {
AppStorage storage s = LibAppStorage.diamondStorage();
uint256 remainingBeansToShip = beansToShip;
ShipmentRoute[] memory shipmentRoutes = s.sys.shipmentRoutes;
ShipmentPlan[] memory shipmentPlans = new ShipmentPlan[](shipmentRoutes.length);
uint256[] memory shipmentAmounts = new uint256[](shipmentRoutes.length);
uint256 totalPoints;
(shipmentPlans, totalPoints) = getShipmentPlans(shipmentRoutes);
// May need to calculate individual stream rewards multiple times, since
// they are dependent on each others caps. Once a cap is reached, excess Beans are
// spread to other streams, proportional to their points.
for (uint256 i; i < shipmentRoutes.length; i++) {
bool capExceeded;
// Calculate the amount of rewards to each stream. Ignores cap and plans with 0 points.
getBeansFromPoints(
shipmentAmounts,
shipmentPlans,
totalPoints,
remainingBeansToShip
);
// Iterate though each stream, checking if cap is exceeded.
for (uint256 j; j < shipmentAmounts.length; j++) {
// If shipment amount exceeds plan cap, adjust plan and totals before recomputing.
if (shipmentAmounts[j] > shipmentPlans[j].cap) {
shipmentAmounts[j] = shipmentPlans[j].cap;
remainingBeansToShip -= shipmentPlans[j].cap;
totalPoints -= shipmentPlans[j].points;
shipmentPlans[j].points = 0;
capExceeded = true;
}
}
// If no cap exceeded, amounts are final.
if (!capExceeded) break;
}
// Ship it.
for (uint256 i; i < shipmentAmounts.length; i++) {
if (shipmentAmounts[i] == 0) continue;
LibReceiving.receiveShipment(
shipmentRoutes[i].recipient,
shipmentAmounts[i],
shipmentRoutes[i].data
);
}
emit Shipped(s.sys.season.current, beansToShip);
}

Impact

Though there is a cap check, it doesn't account the total number of beans shipped till date. Instead what it does it checks cap against amount calculated from the provided beansToShip value.

Thus, any malicious user can call ship function again and again with particularbeansToShip amount which doesn't exceed cap for any plans and distribute beans to these plans.

Doing this will break the protocol invariant as more amount will be shipped than the cap. Also, more the shipped amount, more rewards depositors can get. For example: Silo depositors will get more assets if silo is shipped with exponential amount of tokens.

Tools Used

Manual Analysis

Relevant Links

  1. https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/4e0ad0b964f74a1b4880114f4dd5b339bc69cd3e/protocol/contracts/ecosystem/ShipmentPlanner.sol#L57

  2. https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/4e0ad0b964f74a1b4880114f4dd5b339bc69cd3e/protocol/contracts/libraries/LibShipping.sol#L28

Recommendations

  1. Implement proper access control in the ship() function.

  2. ship() function shouldn't be allowed to be called again and again as the current implementation doesn't check the cap amount with total amount of beans shipped.

Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

0xsandy Submitter
11 months ago
giovannidisiena Auditor
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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