Submission Details
Severity:
Chainlink oracles with decimals more than 8 will let a user borrow more than collateralized
Summary
Value returned from PriceCalculator is not scaled properly. In case chainlink data feed has >8 decimals euros will be too big.
Vulnerability Details
https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/utils/PriceCalculator.sol#L48
return collateralUsd / uint256(eurUsdPrice);
https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/SmartVaultV3.sol#L71
euros += calculator.tokenToEurAvg(token, getAssetBalance(token.symbol, token.addr));
Value returned from PriceCalculator is only correct if collateral's data feed decimals == euroUsd decimals. When collateral's data feed has 18 decimals you would need to divide on 1e18-1e8 to get the correct price
The similar problem is present in canRemoveCollateral and calculateMinimumAmountOut for the same reason.
Impact
Users can borrow much more than collateral. For a feed with 18 decimals it will be 1e10 more
Proof of Concept
A test PoC. Put a js file in
test, ,sol incontacts/utils
// SPDX-License-Identifier: UNLICENSED
import "@chainlink/contracts/src/v0.8/interfaces/AggregatorV3Interface.sol";
// Exactly lik ChainlinkMock but return 18 decimals
contract ChainlinkMock18 is AggregatorV3Interface {
string private desc;
PriceRound[] private prices;
struct PriceRound { uint256 timestamp; int256 price; }
constructor (string memory _desc) {
desc = _desc;
}
function decimals() external pure returns (uint8) { return 18; }
function addPriceRound(uint256 timestamp, int256 price) external {
prices.push(PriceRound(timestamp, price));
}
function setPrice(int256 _price) external {
while (prices.length > 0) prices.pop();
prices.push(PriceRound(block.timestamp - 4 hours, _price));
}
function getRoundData(uint80 _roundId) external view
returns (uint80 roundId, int256 answer, uint256, uint256 updatedAt, uint80) {
roundId = _roundId;
answer = prices[roundId].price;
updatedAt = prices[roundId].timestamp;
}
function latestRoundData() external view
returns (uint80 roundId, int256 answer,uint256, uint256 updatedAt,uint80) {
roundId = uint80(prices.length - 1);
answer = prices[roundId].price;
updatedAt = prices[roundId].timestamp;
}
function description() external view returns (string memory) {
return desc;
}
function version() external view returns (uint256) {
return 1;
}
}
// @ts-check
const { ethers } = require('hardhat');
const { BigNumber } = ethers;
const { DEFAULT_ETH_USD_PRICE, DEFAULT_EUR_USD_PRICE, DEFAULT_COLLATERAL_RATE, PROTOCOL_FEE_RATE, getCollateralOf, ETH, getNFTMetadataContract, fullyUpgradedSmartVaultManager, DEFAULT_WBTC_USD_PRICE } = require('./common');
let VaultManager, Vault, TokenManager, ClEthUsd, EUROs, SwapRouterMock, MockWeth, admin, user, otherUser, protocol;
describe('SmartVault', async () => {
beforeEach(async () => {
[ admin, user, otherUser, protocol ] = await ethers.getSigners();
ClEthUsd = await (await ethers.getContractFactory('ChainlinkMock')).deploy('ETH / USD');
await ClEthUsd.setPrice(DEFAULT_ETH_USD_PRICE);
const ClEurUsd = await (await ethers.getContractFactory('ChainlinkMock')).deploy('EUR / USD');
await ClEurUsd.setPrice(DEFAULT_EUR_USD_PRICE);
EUROs = await (await ethers.getContractFactory('EUROsMock')).deploy();
TokenManager = await (await ethers.getContractFactory('TokenManagerMock')).deploy(ETH, ClEthUsd.address);
const SmartVaultDeployer = await (await ethers.getContractFactory('SmartVaultDeployerV3')).deploy(ETH, ClEurUsd.address);
const SmartVaultIndex = await (await ethers.getContractFactory('SmartVaultIndex')).deploy();
const NFTMetadataGenerator = await (await getNFTMetadataContract()).deploy();
SwapRouterMock = await (await ethers.getContractFactory('SwapRouterMock')).deploy();
MockWeth = await (await ethers.getContractFactory('WETHMock')).deploy();
VaultManager = await fullyUpgradedSmartVaultManager(
DEFAULT_COLLATERAL_RATE, PROTOCOL_FEE_RATE, EUROs.address, protocol.address,
protocol.address, TokenManager.address, SmartVaultDeployer.address,
SmartVaultIndex.address, NFTMetadataGenerator.address, MockWeth.address,
SwapRouterMock.address
);
await SmartVaultIndex.setVaultManager(VaultManager.address);
await EUROs.grantRole(await EUROs.DEFAULT_ADMIN_ROLE(), VaultManager.address);
await VaultManager.connect(user).mint();
const { status } = (await VaultManager.connect(user).vaults())[0];
const { vaultAddress } = status;
Vault = await ethers.getContractAt('SmartVaultV3', vaultAddress);
});
describe('adding collateral', async () => {
it('accepts native currency as collateral', async () => {
const WBTC = await (await ethers.getContractFactory('ERC20Mock'))
.deploy('Wrapped Bitcoin', 'WBTC', 8);
await WBTC.mint(Vault.address, 1e8);
const wbtcPriceAbsolute = BigNumber.from(35_000);
/* Step 1: test 8 decimals */
const normalWbtcUsd = await (await ethers.getContractFactory('ChainlinkMock')).deploy('WBTC / USD');
await normalWbtcUsd.setPrice(wbtcPriceAbsolute.mul(1e8));
await TokenManager.addAcceptedToken(WBTC.address, normalWbtcUsd.address);
let {maxMintable} = await Vault.status();
console.log('maxMintable 8 decimals \t€', maxMintable/1e18)
/* Step 2: test 18 decimals */
const badWbtcUsd = await (await ethers.getContractFactory('ChainlinkMock18')).deploy('WBTC / USD');
await badWbtcUsd.setPrice(wbtcPriceAbsolute.mul(1e8).mul(1e10));
await TokenManager.removeAcceptedToken(ethers.utils.formatBytes32String('WBTC'));
await TokenManager.addAcceptedToken(WBTC.address, badWbtcUsd.address);
({maxMintable} = await Vault.status());
console.log('maxMintable 18 decimals \t€', maxMintable/1e18)
});
});
});
A script. Put it in
scriptsthen runnpx hardhat run scripts/M9-2.js. Note: requests to RPC may take some time, maybe up to 1-2 minutes
Expected output is something like:
[tokenToEurAvg]: 1 PEPE = €10652.41587606
[latestRoundData]: 1 PEPE = €0.00000107
--------------------
[tokenToEurAvg]: 1 ARB = €1.61597849
[latestRoundData]: 1 ARB = €1.61355473
--------------------
coingecko prices { arbitrum: { eur: 1.62 }, pepe: { eur: 0.00000107 } }
// @ts-check
const { ethers, network } = require("hardhat");
const AggregatorV3InterfaceABI = require('@chainlink/contracts/abi/v0.8/AggregatorV3Interface.json');
const assert = require('assert');
// Replace with your Arbitrum RPC URL
const JSON_RPC_PROVIDER = 'https://arbitrum.llamarpc.com';
// eurUsd from readme
const eurUsdPriceFeedAddress = '0xa14d53bc1f1c0f31b4aa3bd109344e5009051a84';
async function main() {
await setUpNetwork();
const priceCalculator = await deployPriceCalculator();
// Not all feeds have 8 decimals, e.g. PEPE have 18 decimals
// https://docs.chain.link/data-feeds/price-feeds/addresses?network=arbitrum&page=1&search=PEPE
const pepe = getPepeToken()
const arb = await getArbToken();
// First we request a price for pepe, `tokenToEurAvg` price is different from latestRoundData
// The error is 1e18/1e8 = 1e10
await printAvgAndCurrentPrice(priceCalculator, pepe);
// Then we request link price for comparison, it's correct for both because it has
// 8 decimals in chainlink
await printAvgAndCurrentPrice(priceCalculator, arb);
// Just for comparison, which prices are correct
console.log(
'coingecko prices',
await (await fetch(
'https://api.coingecko.com/api/v3/simple/price?ids=PEPE,ARBITRUM&vs_currencies=eur'
)).json()
);
}
async function printAvgAndCurrentPrice(priceCalculator, token) {
const PRECISION_TO_NUMBER = 1e8;
const tokenDecimalsMultiplier = ethers.BigNumber.from('10').pow(token.dec);
const tokenClDecimalsMultiplier = ethers.BigNumber.from('10').pow(token.clDec);
const eurClDecimalsMultiplier = ethers.BigNumber.from('10').pow(8);
const eurDecimalsMultiplier = ethers.BigNumber.from('10').pow(18);
// Amount as stored in the contract, with decimals after the whole amount
const amount = ethers.BigNumber.from(1).mul(tokenDecimalsMultiplier);
/* Step 1: average price */
// it has 18 decimals
const tokenEurAveragePriceForAmount = await priceCalculator.tokenToEurAvg(token, amount);
const symbol = ethers.utils.parseBytes32String(token.symbol);
console.log(
`[tokenToEurAvg]:\t`
+ `1 ${symbol} = €`
+ `${
tokenEurAveragePriceForAmount
.mul(PRECISION_TO_NUMBER)
.div(eurDecimalsMultiplier)
.toNumber() / PRECISION_TO_NUMBER
}`
);
/* Step 2: latestRoundData (real price) */
const tokenPriceFeed = new ethers.Contract(token.clAddr, AggregatorV3InterfaceABI, ethers.provider);
const eurUsdPriceFeed = new ethers.Contract(eurUsdPriceFeedAddress, AggregatorV3InterfaceABI, ethers.provider);
// 18 decimals for pepe, 8 for link
const tokenUsdLatestPrice = (await tokenPriceFeed.latestRoundData()).answer;
const eurUsdLatestPrice = (await eurUsdPriceFeed.latestRoundData()).answer;
const decimalsDiff = tokenClDecimalsMultiplier.div(eurClDecimalsMultiplier);
const tokenEurLatestPriceForAmount = tokenUsdLatestPrice
.mul(PRECISION_TO_NUMBER)
.div(decimalsDiff)
.div(eurUsdLatestPrice)
.toNumber() / PRECISION_TO_NUMBER;
console.log(
`[latestRoundData]:\t`
+ `1 ${symbol} = €`
+ `${
tokenEurLatestPriceForAmount
}`
);
// Just to show that it's correct
assert(token.clDec === (await tokenPriceFeed.decimals()));
console.log('-'.repeat(20));
}
async function setUpNetwork() {
await network.provider.request({
method: "hardhat_reset",
params: [{
forking: {
jsonRpcUrl: JSON_RPC_PROVIDER,
},
}],
});
}
async function deployPriceCalculator() {
const PriceCalculator = await ethers.getContractFactory("PriceCalculator");
const priceCalculator = await PriceCalculator.deploy(
ethers.utils.formatBytes32String('ETH'),
eurUsdPriceFeedAddress
);
await priceCalculator.deployed();
return priceCalculator;
}
function getPepeToken() {
return {
symbol: ethers.utils.formatBytes32String('PEPE'),
addr: '0xfc44abE4f62122d31E3fF317d60F7BbcE7e7B7DB',
dec: 18,
clAddr: '0x02DEd5a7EDDA750E3Eb240b54437a54d57b74dBE',
clDec: 18
};
}
async function getArbToken() {
const tokenManager = await ethers.getContractAt(
"ITokenManager",
// Address from readme
'0x33c5A816382760b6E5fb50d8854a61b3383a32a0'
);
return await tokenManager.getToken(
ethers.utils.formatBytes32String('ARB')
);
}
main();
Tools Used
Manual review
Recommended Mitigation Steps
Divide value returned from calculator on ITokenManager.Token.clDec/1e8 where 1e8 is eurUsd's decimals.
Updates
Lead Judging Commences
hrishibhat over 1 year ago
Submission Judgement Published Reason: Non-acceptable severity
Assigned finding tags:
chainlink-decimals
informational/invalid
00xSEV
over 1 year ago
hrishibhat over 1 year ago
Submission Judgement Published Reason: Known issue
Assigned finding tags:
chainlink-decimals
informational/invalid
Prize pool breakdown
Total prize
20,000 USDC
nSLOC
60933 USDC / LOC
17,500 USDC
2,500 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.