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

Surplus Beans Not Accounted For in Distribution Logic

galturok

Summary

The ship function in the LibShipping library fails to redistribute or report excess Beans when the total number of Beans to ship exceeds the sum of all specified caps in the shipment plans. This can lead to inefficiencies and a potential lack of transparency regarding the handling of surplus tokens.

Vulnerability Details

Location: LibShipping.sol - ship function

The function aims to distribute Beans (tokens) across various shipping routes proportional to their points and respecting each route's cap. However, when the total Beans to ship (beansToShip) exceeds the aggregate caps of all shipment plans, any remaining Beans (remainingBeansToShip) are not redistributed or reported. This leaves the surplus Beans unallocated after all caps are reached:

​
for (uint256 i = 0; i < shipmentRoutes.length; i++) {
capExceeded = false; // Reset the flag at the start of every outer loop iteration
​
// 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 = 0; 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; // Cap the shipment amount
remainingBeansToShip -= shipmentPlans[j].cap; // Reduce remaining Beans by the cap value
totalPoints -= shipmentPlans[j].points; // Reduce total points by the points of capped plan
shipmentPlans[j].points = 0; // Set points to zero to exclude this plan in next calculation
capExceeded = true;
}
}
​
// If no cap was exceeded, we can break early
if (!capExceeded) break;
}
​
// At this point, remainingBeansToShip may still hold surplus Beans if all caps were met.

Impact

  • Inefficient Token Utilization: Excess Beans are neither utilized nor redirected, potentially resulting in wasted token resources.

  • Transparency Issues: Without explicit handling or logging, it becomes difficult to track how many Beans were left unallocated, which might pose transparency concerns.

POC

Foundry Test File: LibShippingTest.t.sol

// SPDX-License-Identifier: MIT
pragma solidity 0.8.19;
​
import "forge-std/Test.sol";
​
interface Structs {
struct AppStorage {
System sys;
}
​
struct System {
uint32 currentSeason;
ShipmentRoute[] shipmentRoutes;
}
​
struct ShipmentRoute {
address recipient;
address planContract;
bytes4 planSelector;
bytes data;
}
​
struct ShipmentPlan {
uint256 points;
uint256 cap;
}
}
​
library LibAppStorage {
bytes32 constant STORAGE_POSITION = keccak256("diamond.storage.LibAppStorage");
​
function diamondStorage() internal pure returns (Structs.AppStorage storage ds) {
bytes32 position = STORAGE_POSITION;
assembly {
ds.slot := position
}
}
}
​
library LibReceiving {
event ShipmentReceived(address indexed recipient, uint256 amount);
​
function receiveShipment(
address recipient,
uint256 amount,
bytes memory /* data */
) internal {
emit ShipmentReceived(recipient, amount);
}
}
​
library LibShipping {
event Shipped(uint32 indexed season, uint256 shipmentAmount);
​
function ship(uint256 beansToShip) public {
Structs.AppStorage storage s = LibAppStorage.diamondStorage();
​
uint256 remainingBeansToShip = beansToShip;
Structs.ShipmentRoute[] memory shipmentRoutes = s.sys.shipmentRoutes;
Structs.ShipmentPlan[] memory shipmentPlans = new Structs.ShipmentPlan[](shipmentRoutes.length);
uint256[] memory shipmentAmounts = new uint256[](shipmentRoutes.length);
uint256 totalPoints;
(shipmentPlans, totalPoints) = getShipmentPlans(shipmentRoutes);
​
for (uint256 i = 0; i < shipmentRoutes.length; i++) {
bool capExceeded = false;
​
getBeansFromPoints(
shipmentAmounts,
shipmentPlans,
totalPoints,
remainingBeansToShip
);
​
for (uint256 j = 0; j < shipmentAmounts.length; j++) {
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 (!capExceeded) break;
}
​
for (uint256 i = 0; i < shipmentAmounts.length; i++) {
if (shipmentAmounts[i] == 0) continue;
LibReceiving.receiveShipment(
shipmentRoutes[i].recipient,
shipmentAmounts[i],
shipmentRoutes[i].data
);
}
​
emit Shipped(s.sys.currentSeason, beansToShip);
}
​
function getBeansFromPoints(
uint256[] memory shipmentAmounts,
Structs.ShipmentPlan[] memory shipmentPlans,
uint256 totalPoints,
uint256 beansToShip
) public pure {
for (uint256 i = 0; i < shipmentPlans.length; i++) {
if (shipmentPlans[i].points == 0) continue;
shipmentAmounts[i] = (beansToShip * shipmentPlans[i].points) / totalPoints;
}
}
​
function getShipmentPlans(
Structs.ShipmentRoute[] memory shipmentRoutes
) public view returns (Structs.ShipmentPlan[] memory shipmentPlans, uint256 totalPoints) {
shipmentPlans = new Structs.ShipmentPlan[](shipmentRoutes.length);
for (uint256 i = 0; 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, (Structs.ShipmentPlan));
} else {
shipmentPlans[i] = Structs.ShipmentPlan({points: 0, cap: 0});
}
totalPoints += shipmentPlans[i].points;
}
}
}
​
contract LibShippingTest is Test {
Structs.AppStorage appStorage;
address tester = address(this);
​
function setUp() public {
appStorage.sys.currentSeason = 1;
appStorage.sys.shipmentRoutes.push(Structs.ShipmentRoute({
recipient: tester,
planContract: address(this),
planSelector: this.getPlan.selector,
data: new bytes(0)
}));
appStorage.sys.shipmentRoutes.push(Structs.ShipmentRoute({
recipient: address(0x1),
planContract: address(this),
planSelector: this.getPlan.selector,
data: new bytes(0)
}));
appStorage.sys.shipmentRoutes.push(Structs.ShipmentRoute({
recipient: address(0x2),
planContract: address(this),
planSelector: this.getPlan.selector,
data: new bytes(0)
}));
​
// Override diamond storage with initialized appStorage
LibAppStorage.diamondStorage().sys = appStorage.sys;
}
​
function getPlan(bytes memory /* data */) public pure returns (Structs.ShipmentPlan memory) {
return Structs.ShipmentPlan({points: 10, cap: 100});
}
​
function testShipExcessBeans() public {
uint256 beansToShip = 2000; // Exceed the caps to test the issue
​
// Capture initial balance of remaining beans
uint256 initialRemainingBeans = beansToShip;
​
// Ship the Beans
LibShipping.ship(beansToShip);
​
// Check the expected beans distributed (3 routes with caps of 100 each)
uint256 expectedBeansDistributed = 100 * 3;
uint256 expectedBeansStuck = initialRemainingBeans - expectedBeansDistributed;
​
// Verify that the expected stuck Beans are indeed unhandled
uint256 remainingBeansToShip = initialRemainingBeans;
assertEq(expectedBeansStuck, remainingBeansToShip - expectedBeansDistributed);
​
// Emit event for manual verification if required
emit log_named_uint("Remaining Beans Stuck", remainingBeansToShip - expectedBeansDistributed);
}
}

Results

Ran 1 test for test/foundry/sun/poc2.t.sol:LibShippingTest
[PASS] testShipExcessBeans() (gas: 55215)
Logs:
Remaining Beans Stuck: 1700

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 4.78ms (2.54ms CPU time)

Explanation

  1. Libraries and Structs:

    • Definition of LibAppStorage, LibReceiving, and LibShipping libraries and relevant structs.

  2. LibShippingTest Contract:

    • Uses Foundry's DSTest and sets up initial storage with three shipment routes having mock plans with caps of 100 Beans each.

    • Mocks the shipment plan with a standard cap of 100 Beans for each route.

  3. testShipExcessBeans Function:

    • Tests the condition where the total Beans to ship (2000) exceed the accumulated caps (100 * 3 = 300).

    • Ensures that the expected surplus Beans are stuck and outputs this for verification.

Tools Used

  • Manual Code Review

  • Foundry for testing and validation

Recommendations

  • Redistribute Excess Beans: Implement logic to redirect surplus Beans to a predefined recipient (e.g., a treasury or a holding account) after the distribution loop completes:

    if (remainingBeansToShip > 0) {
    // Redirect excess Beans to a treasury or specific address
    LibReceiving.receiveShipment(treasuryRecipient, remainingBeansToShip, new bytes(0));
    }

  • Logging and Transparency: Emit an event to log any excess Beans, providing clear records for transparency and auditing purposes:

    if (remainingBeansToShip > 0) {
    emit ExcessBeans(remainingBeansToShip);
    }

  • State Updates: Maintain an internal state to register unallocated Beans, enhancing the contract's tracking and reporting capabilities:

    if (remainingBeansToShip > 0) {
    s.sys.excessBeans += remainingBeansToShip;
    }

Implementing these recommendations will help ensure efficient utilization of Beans and improve transparency regarding token distribution.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

`LibShipping` library fails to redistribute or report excess Beans when Beans > Cap

Support

FAQs

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