Skip to content

Commit d8a5c6e

Browse files
danoctaviandanoctavian
committed
fix ynlsd donation attack issue audit-6.1
1 parent b45ee01 commit d8a5c6e

File tree

5 files changed

+127
-30
lines changed

5 files changed

+127
-30
lines changed

‎src/ynLSD.sol

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
77
import {BeaconProxy} from "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol";
88
import {UpgradeableBeacon} from "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol";
99
import {ReentrancyGuardUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/ReentrancyGuardUpgradeable.sol";
10-
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
1110
import {IStrategy} from "./external/eigenlayer/v0.1.0/interfaces/IStrategy.sol";
1211
import {IStrategyManager} from "./external/eigenlayer/v0.1.0/interfaces/IStrategyManager.sol";
1312
import {IDelegationManager} from "./external/eigenlayer/v0.1.0/interfaces/IDelegationManager.sol";
@@ -48,6 +47,8 @@ contract ynLSD is IynLSD, ynBase, ReentrancyGuardUpgradeable, IynLSDEvents {
4847
bytes32 public constant LSD_RESTAKING_MANAGER_ROLE = keccak256("LSD_RESTAKING_MANAGER_ROLE");
4948
bytes32 public constant LSD_STAKING_NODE_CREATOR_ROLE = keccak256("LSD_STAKING_NODE_CREATOR_ROLE");
5049

50+
uint256 public constant BOOTSTRAP_AMOUNT_UNITS = 10;
51+
5152
//--------------------------------------------------------------------------------------
5253
//---------------------------------- VARIABLES ---------------------------------------
5354
//--------------------------------------------------------------------------------------
@@ -96,6 +97,7 @@ contract ynLSD is IynLSD, ynBase, ReentrancyGuardUpgradeable, IynLSDEvents {
9697
address lsdRestakingManager;
9798
address lsdStakingNodeCreatorRole;
9899
address[] pauseWhitelist;
100+
address depositBootstrapper;
99101
}
100102

101103
function initialize(Init memory init)
@@ -137,6 +139,8 @@ contract ynLSD is IynLSD, ynBase, ReentrancyGuardUpgradeable, IynLSDEvents {
137139

138140
_setTransfersPaused(true); // transfers are initially paused
139141
_updatePauseWhitelist(init.pauseWhitelist, true);
142+
143+
_deposit(assets[0], BOOTSTRAP_AMOUNT_UNITS * (10 ** (IERC20Metadata(address(assets[0])).decimals())), init.depositBootstrapper, init.depositBootstrapper);
140144
}
141145

142146
//--------------------------------------------------------------------------------------
@@ -157,7 +161,16 @@ contract ynLSD is IynLSD, ynBase, ReentrancyGuardUpgradeable, IynLSDEvents {
157161
IERC20 asset,
158162
uint256 amount,
159163
address receiver
160-
) external nonReentrant returns (uint256 shares) {
164+
) public nonReentrant returns (uint256 shares) {
165+
return _deposit(asset, amount, receiver, msg.sender);
166+
}
167+
168+
function _deposit(
169+
IERC20 asset,
170+
uint256 amount,
171+
address receiver,
172+
address sender
173+
) internal returns (uint256 shares) {
161174

162175
IStrategy strategy = strategies[asset];
163176
if(address(strategy) == address(0x0)){
@@ -178,9 +191,9 @@ contract ynLSD is IynLSD, ynBase, ReentrancyGuardUpgradeable, IynLSDEvents {
178191

179192
// Transfer assets in after shares are computed since _convertToShares relies on totalAssets
180193
// which inspects asset.balanceOf(address(this))
181-
asset.safeTransferFrom(msg.sender, address(this), amount);
194+
asset.safeTransferFrom(sender, address(this), amount);
182195

183-
emit Deposit(msg.sender, receiver, amount, shares);
196+
emit Deposit(sender, receiver, amount, shares);
184197
}
185198

186199
/**

‎test/foundry/ActorAddresses.sol

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ contract ActorAddresses {
1616
address LSD_RESTAKING_MANAGER;
1717
address STAKING_NODE_CREATOR;
1818
address ORACLE_MANAGER;
19+
address DEPOSIT_BOOTSTRAPER;
1920
}
2021

2122
mapping(uint256 => Actors) public actors;
@@ -33,7 +34,8 @@ contract ActorAddresses {
3334
PAUSE_ADMIN: 0x23618e81E3f5cdF7f54C3d65f7FBc0aBf5B21E8f,
3435
LSD_RESTAKING_MANAGER: 0xa0Ee7A142d267C1f36714E4a8F75612F20a79720,
3536
STAKING_NODE_CREATOR: 0xBcd4042DE499D14e55001CcbB24a551F3b954096,
36-
ORACLE_MANAGER: 0x71bE63f3384f5fb98995898A86B02Fb2426c5788
37+
ORACLE_MANAGER: 0x71bE63f3384f5fb98995898A86B02Fb2426c5788,
38+
DEPOSIT_BOOTSTRAPER: 0xFABB0ac9d68B0B445fB7357272Ff202C5651694a
3739
});
3840

3941
actors[5] = Actors({
@@ -48,7 +50,8 @@ contract ActorAddresses {
4850
PAUSE_ADMIN: 0x23618e81E3f5cdF7f54C3d65f7FBc0aBf5B21E8f,
4951
LSD_RESTAKING_MANAGER: 0xa0Ee7A142d267C1f36714E4a8F75612F20a79720,
5052
STAKING_NODE_CREATOR: 0xBcd4042DE499D14e55001CcbB24a551F3b954096,
51-
ORACLE_MANAGER: 0x71bE63f3384f5fb98995898A86B02Fb2426c5788
53+
ORACLE_MANAGER: 0x71bE63f3384f5fb98995898A86B02Fb2426c5788,
54+
DEPOSIT_BOOTSTRAPER: 0xFABB0ac9d68B0B445fB7357272Ff202C5651694a
5255
});
5356
}
5457

‎test/foundry/integration/IntegrationBaseTest.sol

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -227,19 +227,20 @@ contract IntegrationBaseTest is Test, Utils {
227227
address[] memory pauseWhitelist = new address[](1);
228228
pauseWhitelist[0] = actors.TRANSFER_ENABLED_EOA;
229229

230-
// rETH
231-
assets[0] = IERC20(chainAddresses.lsd.RETH_ADDRESS);
232-
assetsAddresses[0] = chainAddresses.lsd.RETH_ADDRESS;
233-
strategies[0] = IStrategy(chainAddresses.lsd.RETH_STRATEGY_ADDRESS);
234-
priceFeeds[0] = chainAddresses.lsd.RETH_FEED_ADDRESS;
235-
maxAges[0] = uint256(86400);
236230

237231
// stETH
238-
assets[1] = IERC20(chainAddresses.lsd.STETH_ADDRESS);
239-
assetsAddresses[1] = chainAddresses.lsd.STETH_ADDRESS;
240-
strategies[1] = IStrategy(chainAddresses.lsd.STETH_STRATEGY_ADDRESS);
241-
priceFeeds[1] = chainAddresses.lsd.STETH_FEED_ADDRESS;
242-
maxAges[1] = uint256(86400); //one hour
232+
assets[0] = IERC20(chainAddresses.lsd.STETH_ADDRESS);
233+
assetsAddresses[0] = chainAddresses.lsd.STETH_ADDRESS;
234+
strategies[0] = IStrategy(chainAddresses.lsd.STETH_STRATEGY_ADDRESS);
235+
priceFeeds[0] = chainAddresses.lsd.STETH_FEED_ADDRESS;
236+
maxAges[0] = uint256(86400); //one hour
237+
238+
// rETH
239+
assets[1] = IERC20(chainAddresses.lsd.RETH_ADDRESS);
240+
assetsAddresses[1] = chainAddresses.lsd.RETH_ADDRESS;
241+
strategies[1] = IStrategy(chainAddresses.lsd.RETH_STRATEGY_ADDRESS);
242+
priceFeeds[1] = chainAddresses.lsd.RETH_FEED_ADDRESS;
243+
maxAges[1] = uint256(86400);
243244

244245
YieldNestOracle.Init memory oracleInit = YieldNestOracle.Init({
245246
assets: assetsAddresses,
@@ -266,9 +267,18 @@ contract IntegrationBaseTest is Test, Utils {
266267
lsdRestakingManager: actors.LSD_RESTAKING_MANAGER,
267268
lsdStakingNodeCreatorRole: actors.STAKING_NODE_CREATOR,
268269
pauseWhitelist: pauseWhitelist,
269-
pauser: actors.PAUSE_ADMIN
270+
pauser: actors.PAUSE_ADMIN,
271+
depositBootstrapper: actors.DEPOSIT_BOOTSTRAPER
270272
});
271273

274+
vm.deal(actors.DEPOSIT_BOOTSTRAPER, 10000 ether);
275+
276+
vm.prank(actors.DEPOSIT_BOOTSTRAPER);
277+
(bool success, ) = chainAddresses.lsd.STETH_ADDRESS.call{value: 1000 ether}("");
278+
require(success, "ETH transfer failed");
279+
280+
vm.prank(actors.DEPOSIT_BOOTSTRAPER);
281+
IERC20(chainAddresses.lsd.STETH_ADDRESS).approve(address(ynlsd), type(uint256).max);
272282
ynlsd.initialize(init);
273283

274284
vm.prank(actors.STAKING_ADMIN);

‎test/foundry/integration/Upgrades.t.sol

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,8 @@ contract UpgradesTest is IntegrationBaseTest {
210210
lsdRestakingManager: actors.LSD_RESTAKING_MANAGER,
211211
lsdStakingNodeCreatorRole: actors.STAKING_NODE_CREATOR,
212212
pauseWhitelist: pauseWhitelist,
213-
pauser: actors.PAUSE_ADMIN
213+
pauser: actors.PAUSE_ADMIN,
214+
depositBootstrapper: actors.DEPOSIT_BOOTSTRAPER
214215
});
215216

216217
return (init, ynlsd);

‎test/foundry/integration/ynLSD.t.sol

Lines changed: 81 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import {TestLSDStakingNodeV2} from "test/foundry/mocks/TestLSDStakingNodeV2.sol"
1515
import {TestYnLSDV2} from "test/foundry/mocks/TestYnLSDV2.sol";
1616
import {ynBase} from "src/ynBase.sol";
1717

18-
1918
contract ynLSDAssetTest is IntegrationBaseTest {
2019
function testDepositSTETHFailingWhenStrategyIsPaused() public {
2120
IERC20 asset = IERC20(chainAddresses.lsd.STETH_ADDRESS);
@@ -38,7 +37,7 @@ contract ynLSDAssetTest is IntegrationBaseTest {
3837
assets[0] = asset;
3938
amounts[0] = amount;
4039

41-
vm.expectRevert(bytes("BALANCE_EXCEEDED"));
40+
vm.expectRevert(bytes("Pausable: index is paused"));
4241
vm.prank(actors.LSD_RESTAKING_MANAGER);
4342
lsdStakingNode.depositAssetsToEigenlayer(assets, amounts);
4443
}
@@ -47,6 +46,9 @@ contract ynLSDAssetTest is IntegrationBaseTest {
4746
IERC20 stETH = IERC20(chainAddresses.lsd.STETH_ADDRESS);
4847
uint256 amount = 32 ether;
4948

49+
uint256 initialSupply = ynlsd.totalSupply();
50+
uint256 initialTotalAssets = ynlsd.totalAssets();
51+
5052
// Obtain STETH
5153
(bool success, ) = chainAddresses.lsd.STETH_ADDRESS.call{value: amount + 1}("");
5254
require(success, "ETH transfer failed");
@@ -58,15 +60,18 @@ contract ynLSDAssetTest is IntegrationBaseTest {
5860
stETH.approve(address(ynlsd), 32 ether);
5961
ynlsd.deposit(stETH, depositAmount, address(this));
6062

61-
assertEq(ynlsd.balanceOf(address(this)), ynlsd.totalSupply(), "ynlsd balance does not match total supply");
62-
assertTrue((depositAmount - ynlsd.totalAssets()) < 1e18, "Total assets do not match user deposits");
63+
assertEq(ynlsd.balanceOf(address(this)), ynlsd.totalSupply() - initialSupply, "ynlsd balance does not match total supply");
64+
assertTrue((depositAmount - (ynlsd.totalAssets() - initialTotalAssets)) < 1e18, "Total assets do not match user deposits");
6365
assertTrue((depositAmount - ynlsd.balanceOf(address(this))) < 1e18, "Invalid ynLSD Balance");
6466
}
6567

6668
function testDepositSTETHSuccessWithMultipleDeposits() public {
6769
IERC20 stETH = IERC20(chainAddresses.lsd.STETH_ADDRESS);
6870
uint256 amount = 32 ether;
6971

72+
uint256 initialSupply = ynlsd.totalSupply();
73+
uint256 initialTotalAssets = ynlsd.totalAssets();
74+
7075
// Obtain STETH
7176
(bool success, ) = chainAddresses.lsd.STETH_ADDRESS.call{value: amount + 1}("");
7277
require(success, "ETH transfer failed");
@@ -84,8 +89,8 @@ contract ynLSDAssetTest is IntegrationBaseTest {
8489

8590
uint256 totalDeposit = depositAmountOne + depositAmountTwo + depositAmountThree;
8691

87-
assertEq(ynlsd.balanceOf(address(this)), ynlsd.totalSupply(), "ynlsd balance does not match total supply");
88-
assertTrue((totalDeposit - ynlsd.totalAssets()) < 1e18, "Total assets do not match user deposits");
92+
assertEq(ynlsd.balanceOf(address(this)), ynlsd.totalSupply() - initialSupply, "ynlsd balance does not match total supply");
93+
assertTrue((totalDeposit - (ynlsd.totalAssets() - initialTotalAssets)) < 1e18, "Total assets do not match user deposits");
8994
assertTrue(totalDeposit - ynlsd.balanceOf(address(this)) < 1e18, "Invalid ynLSD Balance");
9095
}
9196

@@ -114,17 +119,29 @@ contract ynLSDAssetTest is IntegrationBaseTest {
114119
ynlsd.convertToShares(asset, amount);
115120
}
116121

122+
function testConvertToSharesBootstrapStrategy() public {
123+
vm.prank(actors.STAKING_NODE_CREATOR);
124+
ynlsd.createLSDStakingNode();
125+
uint256[] memory totalAssets = ynlsd.getTotalAssets();
126+
ynlsd.nodes(0);
127+
128+
uint256 bootstrapAmountUnits = ynlsd.BOOTSTRAP_AMOUNT_UNITS() * 1e18 - 1;
129+
assertTrue(compareWithThreshold(totalAssets[0], bootstrapAmountUnits, 1), "Total assets should be equal to bootstrap amount");
130+
}
131+
117132
function testConvertToSharesZeroStrategy() public {
118133
vm.prank(actors.STAKING_NODE_CREATOR);
119134
ynlsd.createLSDStakingNode();
120135
uint256[] memory totalAssets = ynlsd.getTotalAssets();
121136
ynlsd.nodes(0);
122-
assertEq(totalAssets[0], 0, "Total assets should be zero");
137+
138+
assertEq(totalAssets[1], 0, "Total assets should be equal to bootstrap 0");
123139
}
124140

125141
function testGetTotalAssets() public {
142+
uint256 totalAssetsInETH = ynlsd.convertToETH(ynlsd.assets(0), ynlsd.BOOTSTRAP_AMOUNT_UNITS() * 1e18 - 1);
126143
uint256 totalAssets = ynlsd.totalAssets();
127-
assertEq(totalAssets, 0, "Total assets should be zero");
144+
assertTrue(compareWithThreshold(totalAssets, totalAssetsInETH, 1), "Total assets should be equal to bootstrap amount converted to its ETH value");
128145
}
129146

130147
function testLSDWrongStrategy() public {
@@ -142,8 +159,8 @@ contract ynLSDAssetTest is IntegrationBaseTest {
142159
uint256 shares = ynlsd.convertToShares(asset, amount);
143160
(, int256 price, , uint256 timeStamp, ) = assetPriceFeed.latestRoundData();
144161

145-
assertEq(ynlsd.totalAssets(), 0);
146-
assertEq(ynlsd.totalSupply(), 0);
162+
// assertEq(ynlsd.totalAssets(), 0);
163+
// assertEq(ynlsd.totalSupply(), 0);
147164

148165
assertEq(timeStamp > 0, true, "Zero timestamp");
149166
assertEq(price > 0, true, "Zero price");
@@ -164,6 +181,8 @@ contract ynLSDAssetTest is IntegrationBaseTest {
164181
vm.startPrank(unpauser);
165182
pausableStrategyManager.unpause(0);
166183
vm.stopPrank();
184+
185+
uint256 totalAssetsBeforeDeposit = ynlsd.totalAssets();
167186

168187
// Obtain STETH
169188
(bool success, ) = chainAddresses.lsd.STETH_ADDRESS.call{value: amount + 1}("");
@@ -173,6 +192,7 @@ contract ynLSDAssetTest is IntegrationBaseTest {
173192
asset.approve(address(ynlsd), amount);
174193
ynlsd.deposit(asset, amount, address(this));
175194

195+
176196
{
177197
IERC20[] memory assets = new IERC20[](1);
178198
uint256[] memory amounts = new uint256[](1);
@@ -194,7 +214,7 @@ contract ynLSDAssetTest is IntegrationBaseTest {
194214
uint256 expectedBalance = balanceInStrategyForNode * oraclePrice / 1e18;
195215

196216
// Assert that totalAssets reflects the deposit
197-
assertEq(totalAssetsAfterDeposit, expectedBalance, "Total assets do not reflect the deposit");
217+
assertEq(totalAssetsAfterDeposit - totalAssetsBeforeDeposit, expectedBalance, "Total assets do not reflect the deposit");
198218
}
199219

200220
function testPreviewDeposit() public {
@@ -527,4 +547,54 @@ contract ynLSDTransferPauseTest is IntegrationBaseTest {
527547
assertFalse(isFirstAddressWhitelisted, "First new whitelist address was not removed");
528548
assertFalse(isSecondAddressWhitelisted, "Second new whitelist address was not removed");
529549
}
550+
}
551+
552+
553+
contract ynLSDDonationsTest is IntegrationBaseTest {
554+
555+
function testYnLSDdonationToZeroShareAttackResistance() public {
556+
557+
uint INITIAL_AMOUNT = 10_000 ether;
558+
559+
address _alice = makeAddr("Alice");
560+
address _bob = makeAddr("Bob");
561+
562+
IERC20 assetToken = IERC20(chainAddresses.lsd.STETH_ADDRESS);
563+
564+
vm.deal(_alice, INITIAL_AMOUNT);
565+
vm.deal(_bob, INITIAL_AMOUNT);
566+
567+
568+
IERC20 steth = IERC20(chainAddresses.lsd.STETH_ADDRESS);
569+
570+
// get stETH
571+
vm.startPrank(_alice);
572+
(bool success, ) = chainAddresses.lsd.STETH_ADDRESS.call{value: INITIAL_AMOUNT}("");
573+
require(success, "ETH transfer failed");
574+
575+
steth.approve(address(ynlsd), type(uint256).max);
576+
577+
vm.startPrank(_bob);
578+
(success, ) = chainAddresses.lsd.STETH_ADDRESS.call{value: INITIAL_AMOUNT}("");
579+
require(success, "ETH transfer failed");
580+
581+
steth.approve(address(ynlsd), type(uint256).max);
582+
583+
// Front-running part
584+
uint256 bobDepositAmount = INITIAL_AMOUNT / 2;
585+
// Alice knows that Bob is about to deposit INITIAL_AMOUNT*0.5 ATK to the Vault by observing the mempool
586+
vm.startPrank(_alice);
587+
uint256 aliceDepositAmount = 1;
588+
uint256 aliceShares = ynlsd.deposit(assetToken, aliceDepositAmount, _alice);
589+
assertEq(aliceShares, 0); // Since there are boostrap funds, this has no effect
590+
// Try to inflate shares value
591+
assetToken.transfer(address(ynlsd), bobDepositAmount);
592+
vm.stopPrank();
593+
594+
// Check that Bob did not get 0 share when he deposits
595+
vm.prank(_bob);
596+
uint256 bobShares = ynlsd.deposit(assetToken, bobDepositAmount, _bob);
597+
598+
assertGt(bobShares, 1 wei, "Bob's shares should be greater than 1 wei");
599+
}
530600
}

0 commit comments

Comments
 (0)