DeFiFoundrySolidity
16,653 OP
View results
Submission Details
Severity: medium
Valid

Inability of the contract to swap donated/residual WETH tokens to alEth will affect the harvest and report function

Summary

Donated WETH and leftover swap residue cannot be converted back to alETH, allowing an attacker to donate WETH into the contract. This impacts the Report function in the main BASE STRATEGY, as the donated amount is counted as profit but cannot be withdrawn. Users will pay the protocol the alETH equivalent of the WETH, leading to the last person withdrawing not receiving their full balance.

Vulnerability Details

Donated and residue WETH can not be swapped backed to alETH, there are no functions to handle this

/\*\*\
\* @dev Internal function to harvest all rewards, redeploy any idle\
\* funds and return an accurate accounting of all funds currently\
\* held by the Strategy.\
\*\
\* This should do any needed harvesting, rewards selling, accrual,\
\* redepositing etc. to get the most accurate view of current assets.\
\*\
\* NOTE: All applicable assets including loose assets should be\
\* accounted for in this function.\
\*\
\* Care should be taken when relying on oracles or swap values rather\
\* than actual amounts as all Strategy profit/loss accounting will\
\* be done based on this returned value.\
\*\
\* This can still be called post a shutdown, a strategist can check\
\* `TokenizedStrategy.isShutdown()` to decide if funds should be\
\* redeployed or simply realize any profits/losses.\
\*\
\* @return \_totalAssets A trusted and accurate account for the total\
\* amount of 'asset' the strategy currently holds including idle funds.\
\*/\
function _harvestAndReport()
internal
override
returns (uint256 _totalAssets)
{
uint256 claimable = transmuter.getClaimableBalance(address(this));
if (claimable > 0) {
// transmuter.claim(claimable, address(this));
}
// NOTE : we can do this in harvest or can do seperately in tend
// if (underlying.balanceOf(address(this)) > 0) {
// _swapUnderlyingToAsset(underlying.balanceOf(address(this)));
// }
uint256 unexchanged = transmuter.getUnexchangedBalance(address(this));
@audit >>>> // NOTE : possible some dormant WETH that isn't swapped yet
@audit >>>> uint256 underlyingBalance = underlying.balanceOf(address(this));
@audit >>>> _totalAssets = unexchanged + asset.balanceOf(address(this)) + underlyingBalance;
}
/*//////////////////////////////////////////////////////////////
OPTIONAL TO OVERRIDE BY STRATEGIST
//////////////////////////////////////////////////////////////*/
/**
* @notice Gets the max amount of `asset` that can be withdrawn.
* @dev Defaults to an unlimited amount for any address. But can
* be overridden by strategists.
*
* This function will be called before any withdraw or redeem to enforce
* any limits desired by the strategist. This can be used for illiquid
* or sandwichable strategies.
*
* EX:
* return asset.balanceOf(yieldSource);
*
* This does not need to take into account the `_owner`'s share balance
* or conversion rates from shares to assets.
*
* @param . The address that is withdrawing from the strategy.
* @return . The available amount that can be withdrawn in terms of `asset`
*/

The claim function does not swap the entire underlying WETH present in the contract but we swap the amount claimed hence there is no way to swap the residue and donated WETH.

/**
* @dev Function called by keeper to claim WETH from transmuter & swap to alETH at premium
* we ensure that we are always swapping at a premium (i.e. keeper cannot swap at a loss)
* @param _amountClaim The amount of WETH to claim from the transmuter
* @param _minOut The minimum amount of alETH to receive after swap
* @param _path The path to swap WETH to alETH (via Ramses Router)
*/
function claimAndSwap(uint256 _amountClaim, uint256 _minOut, IRamsesRouter.route[] calldata _path) external onlyKeepers {
@audit>>> transmuter.claim(_amountClaim, address(this));
uint256 balBefore = asset.balanceOf(address(this));
@audit>>> _swapUnderlyingToAsset(_amountClaim, _minOut, _path);
uint256 balAfter = asset.balanceOf(address(this));
@audit>>> require((balAfter - balBefore) >= _minOut, "Slippage too high");
transmuter.deposit(asset.balanceOf(address(this)), address(this));
}

Impact

The impact of this issue can be found in harvest function on the main BASE STRATEGY. Calls to obtained the total asset in the contract adds the WETH balance in address(this) allowing an attacker to donate and influence the profit calculation causing an imbalance that can never be rectified since the WETH address(this).balance cannot be independently swapped back to alETH

function report()
external
nonReentrant
onlyKeepers
returns (uint256 profit, uint256 loss)
{
// Cache storage pointer since its used repeatedly.\
StrategyData storage S = \_strategyStorage();
// Tell the strategy to report the real total assets it has.
// It should do all reward selling and redepositing now and
// account for deployed and loose `asset` so we can accurately
// account for all funds including those potentially airdropped
// and then have any profits immediately locked.
@audit>>>> uint256 newTotalAssets = IBaseStrategy(address(this))
.harvestAndReport();
uint256 oldTotalAssets = _totalAssets(S);
// Get the amount of shares we need to burn from previous reports.
uint256 sharesToBurn = _unlockedShares(S);
// Initialize variables needed throughout.
uint256 totalFees;
uint256 protocolFees;
uint256 sharesToLock;
uint256 _profitMaxUnlockTime = S.profitMaxUnlockTime;
// Calculate profit/loss.
if (newTotalAssets > oldTotalAssets) {
// We have a profit.
unchecked {
@audit>>>> profit = newTotalAssets - oldTotalAssets;
}
// We need to get the equivalent amount of shares
// at the current PPS before any minting or burning.
sharesToLock = _convertToShares(S, profit, Math.Rounding.Down);
// Cache the performance fee.
uint16 fee = S.performanceFee;
uint256 totalFeeShares;
// If we are charging a performance fee
if (fee != 0) {
// Asses performance fees.
unchecked {
// Get in `asset` for the event.
@audit>>>> totalFees = (profit * fee) / MAX_BPS;
// And in shares for the payment.
@audit>>>> totalFeeShares = (sharesToLock * fee) / MAX_BPS;
}
// Get the protocol fee config from the factory.
@audit>>>> (
uint16 protocolFeeBps,
address protocolFeesRecipient
) = IFactory(FACTORY).protocol_fee_config();
uint256 protocolFeeShares;
// Check if there is a protocol fee to charge.
if (protocolFeeBps != 0) {
unchecked {
// Calculate protocol fees based on the performance Fees.
@audit>>>> protocolFeeShares =
(totalFeeShares * protocolFeeBps) /
MAX_BPS;
// Need amount in underlying for event.
@audit>>>> protocolFees = (totalFees * protocolFeeBps) / MAX_BPS;
}
// Mint the protocol fees to the recipient.
@audit>>>> _mint(S, protocolFeesRecipient, protocolFeeShares);
}
// Mint the difference to the strategy fee recipient.
unchecked {
@audit>>>> _mint(
S,
S.performanceFeeRecipient,
totalFeeShares - protocolFeeShares
);
}
}
// Check if we are locking profit.
if (_profitMaxUnlockTime != 0) {
// lock (profit - fees)
unchecked {
sharesToLock -= totalFeeShares;
}
// If we are burning more than re-locking.
if (sharesToBurn > sharesToLock) {
// Burn the difference
unchecked {
_burn(S, address(this), sharesToBurn - sharesToLock);
}
} else if (sharesToLock > sharesToBurn) {
// Mint the shares to lock the strategy.
unchecked {
_mint(S, address(this), sharesToLock - sharesToBurn);
}
}
}
} else {
// Expect we have a loss.
unchecked {
loss = oldTotalAssets - newTotalAssets;
}
// Check in case `else` was due to being equal.
if (loss != 0) {
// We will try and burn the unlocked shares and as much from any
// pending profit still unlocking to offset the loss to prevent any PPS decline post report.
sharesToBurn = Math.min(
// Cannot burn more than we have.
S.balances[address(this)],
// Try and burn both the shares already unlocked and the amount for the loss.
_convertToShares(S, loss, Math.Rounding.Down) + sharesToBurn
);
}
// Check if there is anything to burn.
if (sharesToBurn != 0) {
_burn(S, address(this), sharesToBurn);
}
}
// Update unlocking rate and time to fully unlocked.
uint256 totalLockedShares = S.balances[address(this)];
if (totalLockedShares != 0) {
uint256 previouslyLockedTime;
uint96 _fullProfitUnlockDate = S.fullProfitUnlockDate;
// Check if we need to account for shares still unlocking.
if (_fullProfitUnlockDate > block.timestamp) {
unchecked {
// There will only be previously locked shares if time remains.
// We calculate this here since it should be rare.
previouslyLockedTime =
(_fullProfitUnlockDate - block.timestamp) *
(totalLockedShares - sharesToLock);
}
}
// newProfitLockingPeriod is a weighted average between the remaining
// time of the previously locked shares and the profitMaxUnlockTime.
uint256 newProfitLockingPeriod = (previouslyLockedTime +
sharesToLock *
_profitMaxUnlockTime) / totalLockedShares;
// Calculate how many shares unlock per second.
S.profitUnlockingRate =
(totalLockedShares * MAX_BPS_EXTENDED) /
newProfitLockingPeriod;
// Calculate how long until the full amount of shares is unlocked.
S.fullProfitUnlockDate = uint96(
block.timestamp + newProfitLockingPeriod
);
} else {
// Only setting this to 0 will turn in the desired effect,
// no need to update profitUnlockingRate.
S.fullProfitUnlockDate = 0;
}
// Update the new total assets value.
S.totalAssets = newTotalAssets;
S.lastReport = uint96(block.timestamp);
// Emit event with info
emit Reported(
profit,
loss,
protocolFees, // Protocol fees
totalFees - protocolFees // Performance Fees
);
}

Tools Used

Manual Review

Recommendations

Implement a way to swap the entire address(this) WETH balance of the contract to alETH.

Updates

Appeal created

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Dormant WETH in the contract will never be swapped back to alETH

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Dormant WETH is not properly treated

Support

FAQs

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