Beanstalk Part 3
Beanstalk
DeFiHardhat
21,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Not Increasing and Decreasing `s.recapitalisation` When Contract Owner Calls `addMigratedUnderlying()` and `beginBarnRaiseMigration()` for UNRIPE_LP

hunter_w3b

Summary

The functions addUnderlying and removeUnderlying are designed to adjust the s.recapitalized value when the token involved is UNRIPE_LP. However, admin functions directly call incrementUnderlying and decrementUnderlying without appropriately updating the s.recapitalized value. The functions UnripeFacet:addMigratedUnderlying() and LibFertilizer:beginBarnRaiseMigration() fail to adjust s.recapitalized when they modify the underlying value of UNRIPE_LP. This oversight leads to a reduced value for unchopped UNRIPE_LP, resulting in unfair payouts for token holders.

Vulnerability Details

The value of s.recapitalized represents the total value that has been recapitalized. This value is crucial for ensuring that the holders of UNRIPE_LP tokens receive the correct returns.

The bug is discovered previously showed on Immunefi that not updating s.recapitalized correctly led to reduced value for UNRIPE_LP holders.

A similar issue exists with the admin functions

  1. addMigratedUnderlying:

This function allows the contract owner to add underlying tokens to the protocol without updating the s.recapitalized value. The function calls LibUnripe::incrementUnderlying without ensuring the s.recapitalized value is adjusted accordingly.

This can lead to an inconsistency where the s.recapitalized value does not reflect the actual state of the underlying tokens, affecting the value distribution for UNRIPE_LP holders.

function addMigratedUnderlying(
address unripeToken,
uint256 amount
) external payable nonReentrant {
LibDiamond.enforceIsContractOwner();
IERC20(s.u[unripeToken].underlyingToken).safeTransferFrom(
msg.sender,
address(this),
amount
);
@>> LibUnripe.incrementUnderlying(unripeToken, amount);
}

  1. beginBarnRaiseMigration:

This function is used during the barn raise migration process and decrements the underlying value of UNRIPE_LP without adjusting s.recapitalized in LibUnripe::decrementUnderlying() function call is called without updating s.recapitalized. This results in an incorrect s.recapitalized value, leading to premature recapitalization and reduced value for remaining UNRIPE_LP holders.

function beginBarnRaiseMigration(address well) external {
LibDiamond.enforceIsOwnerOrContract();
LibFertilizer.beginBarnRaiseMigration(well);
}

The called the LibUnripe.decrementUnderlying(C.UNRIPE_LP, balanceOfUnderlying); without change the recapped

function beginBarnRaiseMigration(address well) internal {
AppStorage storage s = LibAppStorage.diamondStorage();
require(well.isWell(), "Fertilizer: Not a Whitelisted Well.");
​
// The Barn Raise only supports 2 token Wells where 1 token is Bean and the
// other is supported by the Lib Usd Oracle.
IERC20[] memory tokens = IWell(well).tokens();
require(tokens.length == 2, "Fertilizer: Well must have 2 tokens.");
require(
tokens[0] == C.bean() || tokens[1] == C.bean(),
"Fertilizer: Well must have BEAN."
);
// Check that Lib Usd Oracle supports the non-Bean token in the Well.
LibUsdOracle.getTokenPrice(address(tokens[tokens[0] == C.bean() ? 1 : 0]));
​
uint256 balanceOfUnderlying = s.u[C.UNRIPE_LP].balanceOfUnderlying;
IERC20(s.u[C.UNRIPE_LP].underlyingToken).safeTransfer(
LibDiamond.diamondStorage().contractOwner,
balanceOfUnderlying
);
@>> LibUnripe.decrementUnderlying(C.UNRIPE_LP, balanceOfUnderlying);
LibUnripe.switchUnderlyingToken(C.UNRIPE_LP, well);
}

Tools Used

Manual Review

Recommendations

UnripeFacet:addMigratedUnderlying() and LibFertilizer::beginBarnRaiseMigration() must call LibUnripe::addUnderlying() LibUnripe::removeUnderlying() instead of LibUnripe::decrementUnderlying, LibUnripe::incrementUnderlying to recapped the s.recapitalized.

function addUnderlying(address token, uint256 underlying) internal {
AppStorage storage s = LibAppStorage.diamondStorage();
if (token == C.UNRIPE_LP) {
uint256 recapped = underlying.mul(s.recapitalized).div(
s.u[C.UNRIPE_LP].balanceOfUnderlying
);
s.recapitalized = s.recapitalized.add(recapped);
}
incrementUnderlying(token, underlying);
}
function removeUnderlying(address token, uint256 underlying) internal {
AppStorage storage s = LibAppStorage.diamondStorage();
if (token == C.UNRIPE_LP) {// @note
uint256 recapped = underlying.mul(s.recapitalized).div(s.u[C.UNRIPE_LP].balanceOfUnderlying);
s.recapitalized = s.recapitalized.sub(recapped); // recapitalized is subtracted when we remove underlying from
}
decrementUnderlying(token, underlying);
}
Updates

Lead Judging Commences

giovannidisiena Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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