The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Missing "freshness" check on price data from Chainlink oracle may result in incorrect rewards calculation

spacelord47

Summary

During SmartVaultV3 liquidations, Protocol distributes assets, locked in the vault, among all stakers.
Chainlink Data Feed used in the rewards calculation logic, but this logic missing important validation steps to confirm the price data is not "stale".

Issue Details

When using a Chainlink price oracle, the latestRoundData function returns not only the price of asset, but also includes additional metadata which can be used to validate the "staleness" of price. If there is a problem with Chainlink starting a new round, omitting answer validation may result in "stale" price being used in calculations.

Besides that, since the protocol operates on Arbitrum One, it's important to check Arbitrum Sequencer is active, as stated in Chainlink docs.

Impact

Using outdated price data will result in incorrect rewards distribution. Depending on the received price, either stakers, or Protocol will receive less assets than expected.

PoC

Affected code:

contracts/LiquidationPool.sol

217: if (asset.amount > 0) {
218: (,int256 assetPriceUsd,,,) = Chainlink.AggregatorV3Interface(asset.token.clAddr).latestRoundData();
219: uint256 _portion = asset.amount * _positionStake / stakeTotal;
220: uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd)

Recommendation

  1. Consider integrating the following patch, which adds proper validation for Chainlink data and confirms the state of sequencer:

diff --git a/contracts/LiquidationPoolManager.sol b/contracts/LiquidationPoolManager.sol
--- a/contracts/LiquidationPoolManager.sol (revision c12272f2eec533019f2d255ab690f6892027f112)
+++ b/contracts/LiquidationPoolManager.sol (date 1704294208863)
@@ -19,8 +19,8 @@
uint32 public poolFeePercentage;
- constructor(address _TST, address _EUROs, address _smartVaultManager, address _eurUsd, address payable _protocol, uint32 _poolFeePercentage) {
- pool = address(new LiquidationPool(_TST, _EUROs, _eurUsd, ISmartVaultManager(_smartVaultManager).tokenManager()));
+ constructor(address _TST, address _EUROs, address _smartVaultManager, address _eurUsd, address payable _protocol, uint32 _poolFeePercentage, address _sequencer) {
+ pool = address(new LiquidationPool(_TST, _EUROs, _eurUsd, ISmartVaultManager(_smartVaultManager).tokenManager(), _sequencer));
TST = _TST;
EUROs = _EUROs;
smartVaultManager = _smartVaultManager;
diff --git a/contracts/LiquidationPool.sol b/contracts/LiquidationPool.sol
--- a/contracts/LiquidationPool.sol (revision c12272f2eec533019f2d255ab690f6892027f112)
+++ b/contracts/LiquidationPool.sol (date 1704294312350)
@@ -17,6 +17,10 @@
address private immutable EUROs;
address private immutable eurUsd;
+ Chainlink.AggregatorV3Interface private immutable sequencerUptimeFeed;
+ /// @notice L2 Sequencer grace period
+ uint256 private constant GRACE_PERIOD_TIME = 3600;
+
address[] public holders;
mapping(address => Position) private positions;
mapping(bytes => uint256) private rewards;
@@ -28,12 +32,13 @@
struct Reward { bytes32 symbol; uint256 amount; uint8 dec; }
struct PendingStake { address holder; uint256 createdAt; uint256 TST; uint256 EUROs; }
- constructor(address _TST, address _EUROs, address _eurUsd, address _tokenManager) {
+ constructor(address _TST, address _EUROs, address _eurUsd, address _tokenManager, address _sequencer) {
TST = _TST;
EUROs = _EUROs;
eurUsd = _eurUsd;
tokenManager = _tokenManager;
manager = payable(msg.sender);
+ sequencerUptimeFeed = Chainlink.AggregatorV3Interface(_sequencer);
}
modifier onlyManager {
@@ -79,7 +84,7 @@
}
}
}
-
+
function position(address _holder) external view returns(Position memory _position, Reward[] memory _rewards) {
_position = positions[_holder];
(uint256 _pendingTST, uint256 _pendingEUROs) = holderPendingStakes(_holder);
@@ -173,7 +178,7 @@
require(_sent);
} else {
IERC20(_token.addr).transfer(msg.sender, _rewardAmount);
- }
+ }
}
}
@@ -204,7 +209,7 @@
function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {
consolidatePendingStakes();
- (,int256 priceEurUsd,,,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
+ int256 priceEurUsd = getPriceFeedAnswer(eurUsd);
uint256 stakeTotal = getStakeTotal();
uint256 burnEuros;
uint256 nativePurchased;
@@ -215,7 +220,7 @@
for (uint256 i = 0; i < _assets.length; i++) {
ILiquidationPoolManager.Asset memory asset = _assets[i];
if (asset.amount > 0) {
- (,int256 assetPriceUsd,,,) = Chainlink.AggregatorV3Interface(asset.token.clAddr).latestRoundData();
+ int256 assetPriceUsd = getPriceFeedAnswer(asset.token.clAddr);
uint256 _portion = asset.amount * _positionStake / stakeTotal;
uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd)
* _hundredPC / _collateralRate;
@@ -239,4 +244,25 @@
if (burnEuros > 0) IEUROs(EUROs).burn(address(this), burnEuros);
returnUnpurchasedNative(_assets, nativePurchased);
}
+
+ function getPriceFeedAnswer(address _priceFeedAddr) internal view returns (int256) {
+ (uint80 roundId, int256 answer,, uint256 updatedAt, uint80 answeredInRound) =
+ Chainlink.AggregatorV3Interface(_priceFeedAddr).latestRoundData();
+
+ require(isSequencerActive(), "sequencer_down");
+ require(answer > 0, "invalid_price");
+ require(updatedAt != 0, "incomplete_round");
+ require(answeredInRound >= roundId, "stale_price");
+
+ return answer;
+ }
+
+ function isSequencerActive() internal view returns (bool) {
+ (, int256 answer, uint256 startedAt,,) = sequencerUptimeFeed.latestRoundData();
+
+ if (block.timestamp - startedAt <= GRACE_PERIOD_TIME || answer == 1)
+ return false;
+
+ return true;
+ }
}
​

  1. Also, the following patch adds ChainlinkSequencerMock contract to be used in tests:

diff --git a/test/liquidationPool.js b/test/liquidationPool.js
--- a/test/liquidationPool.js (revision c12272f2eec533019f2d255ab690f6892027f112)
+++ b/test/liquidationPool.js (date 1704294208863)
@@ -14,10 +14,11 @@
EUROs = await (await ethers.getContractFactory('EUROsMock')).deploy();
const EurUsd = await (await ethers.getContractFactory('ChainlinkMock')).deploy('EUR / USD');
await EurUsd.setPrice(DEFAULT_EUR_USD_PRICE);
+ const SequencerFeed = await (await ethers.getContractFactory('ChainlinkSequencerMock')).deploy();
const { TokenManager } = await mockTokenManager();
MockSmartVaultManager = await (await ethers.getContractFactory('MockSmartVaultManager')).deploy(DEFAULT_COLLATERAL_RATE, TokenManager.address);
LiquidationPoolManager = await (await ethers.getContractFactory('LiquidationPoolManager')).deploy(
- TST.address, EUROs.address, MockSmartVaultManager.address, EurUsd.address, Protocol.address, POOL_FEE_PERCENTAGE
+ TST.address, EUROs.address, MockSmartVaultManager.address, EurUsd.address, Protocol.address, POOL_FEE_PERCENTAGE, SequencerFeed.address
);
LiquidationPool = await ethers.getContractAt('LiquidationPool', await LiquidationPoolManager.pool());
await EUROs.grantRole(await EUROs.BURNER_ROLE(), LiquidationPool.address)
diff --git a/test/liquidationPoolManager.js b/test/liquidationPoolManager.js
--- a/test/liquidationPoolManager.js (revision c12272f2eec533019f2d255ab690f6892027f112)
+++ b/test/liquidationPoolManager.js (date 1704294208863)
@@ -16,10 +16,11 @@
SmartVaultManager = await (await ethers.getContractFactory('MockSmartVaultManager')).deploy(DEFAULT_COLLATERAL_RATE, TokenManager.address);
const EurUsd = await (await ethers.getContractFactory('ChainlinkMock')).deploy('EUR/USD'); // $1.06
+ const SequencerFeed = await (await ethers.getContractFactory('ChainlinkSequencerMock')).deploy();
await EurUsd.setPrice(DEFAULT_EUR_USD_PRICE);
LiquidationPoolManagerContract = await ethers.getContractFactory('LiquidationPoolManager');
LiquidationPoolManager = await LiquidationPoolManagerContract.deploy(
- TST.address, EUROs.address, SmartVaultManager.address, EurUsd.address, Protocol.address, POOL_FEE_PERCENTAGE
+ TST.address, EUROs.address, SmartVaultManager.address, EurUsd.address, Protocol.address, POOL_FEE_PERCENTAGE, SequencerFeed.address
);
LiquidationPool = await ethers.getContractAt('LiquidationPool', await LiquidationPoolManager.pool());
await EUROs.grantRole(await EUROs.BURNER_ROLE(), LiquidationPool.address);
diff --git a/contracts/utils/ChainlinkSequencerMock.sol b/contracts/utils/ChainlinkSequencerMock.sol
new file mode 100644
--- /dev/null (date 1704294208867)
+++ b/contracts/utils/ChainlinkSequencerMock.sol (date 1704294208867)
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity 0.8.17;
+
+import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";
+
+contract ChainlinkSequencerMock is AggregatorV3Interface {
+ function decimals() external pure returns (uint8) { return 0; }
+
+ function getRoundData(uint80 _roundId) external view
+ returns (uint80 , int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) {
+ }
+
+ function latestRoundData() external view
+ returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) {
+ roundId = 1;
+ answer = 0;
+ startedAt = block.timestamp - 3601;
+ updatedAt = block.timestamp;
+ answeredInRound = 1;
+ }
+
+ function description() external pure returns (string memory) {
+ return "L2 Sequencer Uptime Status Feed";
+ }
+
+ function version() external pure returns (uint256) {
+ return 1;
+ }
+}
Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Arbitrum-sequncer

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

Arbitrum-sequncer

Support

FAQs

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