Line of code
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/libraries/LibShipping.sol#L28
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/libraries/LibShipping.sol#L105
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/beanstalk/sun/SeasonFacet/Sun.sol#L46
Summary
if there are no valid shipment plans, the newly minted BEAN's deltaB is locked and not distributed.
Vulnerability Details
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);
when calling the ship function above, the code attempts to query then compose a shipping plan. This is done by a call to the function getShipmentPlans. If we look at the function getShipmentPlans
we can observe the following.
function getShipmentPlans(
ShipmentRoute[] memory shipmentRoutes
) public view returns (ShipmentPlan[] memory shipmentPlans, uint256 totalPoints) {
shipmentPlans = new ShipmentPlan[](shipmentRoutes.length);
for (uint256 i; i < shipmentRoutes.length; i++) {
(bool success, bytes memory returnData) = shipmentRoutes[i].planContract.staticcall(
abi.encodeWithSelector(shipmentRoutes[i].planSelector, shipmentRoutes[i].data)
);
if (success) {
shipmentPlans[i] = abi.decode(returnData, (ShipmentPlan));
} else {
shipmentPlans[i] = ShipmentPlan({points: 0, cap: 0});
}
totalPoints += shipmentPlans[i].points;
}
}
as we can see, if the shipment router querys fails for whatever reason, instead of revert, the shippment plan is set to 0 point and cap 0
then later this will cause no shippment to occur because of the values both being set to 0 and we will instead continue.
for (uint256 i; i < shipmentAmounts.length; i++) {
if (shipmentAmounts[i] == 0) continue;
LibReceiving.receiveShipment(
shipmentRoutes[i].recipient,
shipmentAmounts[i],
shipmentRoutes[i].data
);
}
then the newly minted deltaB will be locked because of the incorrect logic.
function stepSun(int256 deltaB, uint256 caseId) internal {
if (deltaB > 0) {
uint256 priorHarvestable = s.sys.fields[s.sys.activeField].harvestable;
C.bean().mint(address(this), uint256(deltaB));
LibShipping.ship(uint256(deltaB));
setSoilAbovePeg(s.sys.fields[s.sys.activeField].harvestable - priorHarvestable, caseId);
s.sys.season.abovePeg = true;
}
above we can observe a call to ship in the function stepSun which will cause the locking of deltaB.
Impact
newly minted BEAN's deltaB is locked and not distributed
Tools Used
Manual review
Recommendations
add a check: if all shipment router fails, revert the transaction instead of setting shipmentPlan points and cap to 0
