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

Possible retrieval of manipulated prices in `getUsdPrice()`

tigerfrake

Summary

The LibUsdOracle.getUsdPrice() returns the price of a given token in USD with the option of using a lookback. (Usd:token Price) lookback should be 0 if the instantaneous price is desired. Otherwise, it should be the TWAP lookback in seconds.

According to this function's comments:

If using a non-zero lookback, it is recommended to use a substantially large `lookback` (> 900 seconds) to protect against manipulation.

However, the function doesn't implement the recommendation specified in the comment regarding the use of a non-zero lookback.

Vulnerability Details

  • https://github.com/Cyfrin/2024-04-beanstalk-2/blob/27ff8c87c9164c1fbff054be5f22e56f86cdf127/protocol/contracts/libraries/Oracle/LibUsdOracle.sol#L34-L46

function getUsdPrice(address token, uint256 lookback) internal view returns (uint256) {
if (token == C.WETH) {
uint256 ethUsdPrice = LibEthUsdOracle.getEthUsdPrice(lookback);
if (ethUsdPrice == 0) return 0;
return uint256(1e24).div(ethUsdPrice);
}
if (token == C.WSTETH) {
uint256 wstethUsdPrice = LibWstethUsdOracle.getWstethUsdPrice(lookback);
if (wstethUsdPrice == 0) return 0;
return uint256(1e24).div(wstethUsdPrice);
}
revert("Oracle: Token not supported.");
}

After retrieving the USD price from the respective oracle function, the function checks if the returned price is zero. If it's zero, it returns zero as the USD price of the token.
If the price is not zero, it calculates the USD price of the token by dividing 1e24 by the retrieved price.
However, the USD price returned may be a manipulated figure.

Impact

Malicious actors can exploit the absence of the minimum lookback period to manipulate recent price data, influencing the calculated USD prices of tokens. This manipulation can lead to inaccurate representations of token values or calculations within the protocol.

Tools Used

Manual Review

Recommendations

Adjust the function to accept a substantially large lookback parameter and pass it to the oracle functions.

function getUsdPrice(address token, uint256 lookback) internal view returns (uint256) {
require(lookback > 900, "Lookback should be greater than 900 seconds"); // @audit Protected against manipulation
if (token == C.WETH) {
uint256 ethUsdPrice = LibEthUsdOracle.getEthUsdPrice(lookback);
if (ethUsdPrice == 0) return 0;
return uint256(1e24).div(ethUsdPrice);
}
if (token == C.WSTETH) {
uint256 wstethUsdPrice = LibWstethUsdOracle.getWstethUsdPrice(lookback);
if (wstethUsdPrice == 0) return 0;
return uint256(1e24).div(wstethUsdPrice);
}
revert("Oracle: Token not supported.");
}

With this modification, the function now checks if the lookback parameter is greater than 900 seconds before proceeding to retrieve the USD price from the respective oracles. This ensures that a substantially large lookback is used to protect against manipulation, as recommended in the comment.

Updates

Lead Judging Commences

giovannidisiena Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice
0x11singh99 Judge
over 1 year ago
giovannidisiena Lead Judge
over 1 year ago
giovannidisiena Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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