Skip to content

Commit

Permalink
fix ynlsd donation attack issue audit-6.1
Browse files Browse the repository at this point in the history
  • Loading branch information
@danoctavian
danoctavian committed Mar 22, 2024
1 parent b45ee01 commit 7fd8daf
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 30 deletions.
21 changes: 17 additions & 4 deletions src/ynLSD.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";
import {BeaconProxy} from "@openzeppelin/contracts/proxy/beacon/BeaconProxy.sol";
import {UpgradeableBeacon} from "@openzeppelin/contracts/proxy/beacon/UpgradeableBeacon.sol";
import {ReentrancyGuardUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/ReentrancyGuardUpgradeable.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {IStrategy} from "./external/eigenlayer/v0.1.0/interfaces/IStrategy.sol";
import {IStrategyManager} from "./external/eigenlayer/v0.1.0/interfaces/IStrategyManager.sol";
import {IDelegationManager} from "./external/eigenlayer/v0.1.0/interfaces/IDelegationManager.sol";
Expand Down Expand Up @@ -48,6 +47,8 @@ contract ynLSD is IynLSD, ynBase, ReentrancyGuardUpgradeable, IynLSDEvents {
bytes32 public constant LSD_RESTAKING_MANAGER_ROLE = keccak256("LSD_RESTAKING_MANAGER_ROLE");
bytes32 public constant LSD_STAKING_NODE_CREATOR_ROLE = keccak256("LSD_STAKING_NODE_CREATOR_ROLE");

uint256 public constant BOOTSTRAP_AMOUNT_UNITS = 10;

//--------------------------------------------------------------------------------------
//---------------------------------- VARIABLES ---------------------------------------
//--------------------------------------------------------------------------------------
Expand Down Expand Up @@ -96,6 +97,7 @@ contract ynLSD is IynLSD, ynBase, ReentrancyGuardUpgradeable, IynLSDEvents {
address lsdRestakingManager;
address lsdStakingNodeCreatorRole;
address[] pauseWhitelist;
address depositBootstrapper;
}

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

_setTransfersPaused(true); // transfers are initially paused
_updatePauseWhitelist(init.pauseWhitelist, true);

_deposit(assets[0], BOOTSTRAP_AMOUNT_UNITS * (10 ** (IERC20Metadata(address(assets[0])).decimals())), init.depositBootstrapper, init.depositBootstrapper);
}

//--------------------------------------------------------------------------------------
Expand All @@ -157,7 +161,16 @@ contract ynLSD is IynLSD, ynBase, ReentrancyGuardUpgradeable, IynLSDEvents {
IERC20 asset,
uint256 amount,
address receiver
) external nonReentrant returns (uint256 shares) {
) public nonReentrant returns (uint256 shares) {
return _deposit(asset, amount, receiver, msg.sender);
}

function _deposit(
IERC20 asset,
uint256 amount,
address receiver,
address sender
) internal returns (uint256 shares) {

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

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

emit Deposit(msg.sender, receiver, amount, shares);
emit Deposit(sender, receiver, amount, shares);
}

/**
Expand Down
7 changes: 5 additions & 2 deletions test/foundry/ActorAddresses.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ contract ActorAddresses {
address LSD_RESTAKING_MANAGER;
address STAKING_NODE_CREATOR;
address ORACLE_MANAGER;
address DEPOSIT_BOOTSTRAPER;
}

mapping(uint256 => Actors) public actors;
Expand All @@ -33,7 +34,8 @@ contract ActorAddresses {
PAUSE_ADMIN: 0x23618e81E3f5cdF7f54C3d65f7FBc0aBf5B21E8f,
LSD_RESTAKING_MANAGER: 0xa0Ee7A142d267C1f36714E4a8F75612F20a79720,
STAKING_NODE_CREATOR: 0xBcd4042DE499D14e55001CcbB24a551F3b954096,
ORACLE_MANAGER: 0x71bE63f3384f5fb98995898A86B02Fb2426c5788
ORACLE_MANAGER: 0x71bE63f3384f5fb98995898A86B02Fb2426c5788,
DEPOSIT_BOOTSTRAPER: 0xFABB0ac9d68B0B445fB7357272Ff202C5651694a
});

actors[5] = Actors({
Expand All @@ -48,7 +50,8 @@ contract ActorAddresses {
PAUSE_ADMIN: 0x23618e81E3f5cdF7f54C3d65f7FBc0aBf5B21E8f,
LSD_RESTAKING_MANAGER: 0xa0Ee7A142d267C1f36714E4a8F75612F20a79720,
STAKING_NODE_CREATOR: 0xBcd4042DE499D14e55001CcbB24a551F3b954096,
ORACLE_MANAGER: 0x71bE63f3384f5fb98995898A86B02Fb2426c5788
ORACLE_MANAGER: 0x71bE63f3384f5fb98995898A86B02Fb2426c5788,
DEPOSIT_BOOTSTRAPER: 0xFABB0ac9d68B0B445fB7357272Ff202C5651694a
});
}

Expand Down
34 changes: 22 additions & 12 deletions test/foundry/integration/IntegrationBaseTest.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,19 +227,20 @@ contract IntegrationBaseTest is Test, Utils {
address[] memory pauseWhitelist = new address[](1);
pauseWhitelist[0] = actors.TRANSFER_ENABLED_EOA;

// rETH
assets[0] = IERC20(chainAddresses.lsd.RETH_ADDRESS);
assetsAddresses[0] = chainAddresses.lsd.RETH_ADDRESS;
strategies[0] = IStrategy(chainAddresses.lsd.RETH_STRATEGY_ADDRESS);
priceFeeds[0] = chainAddresses.lsd.RETH_FEED_ADDRESS;
maxAges[0] = uint256(86400);

// stETH
assets[1] = IERC20(chainAddresses.lsd.STETH_ADDRESS);
assetsAddresses[1] = chainAddresses.lsd.STETH_ADDRESS;
strategies[1] = IStrategy(chainAddresses.lsd.STETH_STRATEGY_ADDRESS);
priceFeeds[1] = chainAddresses.lsd.STETH_FEED_ADDRESS;
maxAges[1] = uint256(86400); //one hour
assets[0] = IERC20(chainAddresses.lsd.STETH_ADDRESS);
assetsAddresses[0] = chainAddresses.lsd.STETH_ADDRESS;
strategies[0] = IStrategy(chainAddresses.lsd.STETH_STRATEGY_ADDRESS);
priceFeeds[0] = chainAddresses.lsd.STETH_FEED_ADDRESS;
maxAges[0] = uint256(86400); //one hour

// rETH
assets[1] = IERC20(chainAddresses.lsd.RETH_ADDRESS);
assetsAddresses[1] = chainAddresses.lsd.RETH_ADDRESS;
strategies[1] = IStrategy(chainAddresses.lsd.RETH_STRATEGY_ADDRESS);
priceFeeds[1] = chainAddresses.lsd.RETH_FEED_ADDRESS;
maxAges[1] = uint256(86400);

YieldNestOracle.Init memory oracleInit = YieldNestOracle.Init({
assets: assetsAddresses,
Expand All @@ -266,9 +267,18 @@ contract IntegrationBaseTest is Test, Utils {
lsdRestakingManager: actors.LSD_RESTAKING_MANAGER,
lsdStakingNodeCreatorRole: actors.STAKING_NODE_CREATOR,
pauseWhitelist: pauseWhitelist,
pauser: actors.PAUSE_ADMIN
pauser: actors.PAUSE_ADMIN,
depositBootstrapper: actors.DEPOSIT_BOOTSTRAPER
});

vm.deal(actors.DEPOSIT_BOOTSTRAPER, 10000 ether);

vm.prank(actors.DEPOSIT_BOOTSTRAPER);
(bool success, ) = chainAddresses.lsd.STETH_ADDRESS.call{value: 1000 ether}("");
require(success, "ETH transfer failed");

vm.prank(actors.DEPOSIT_BOOTSTRAPER);
IERC20(chainAddresses.lsd.STETH_ADDRESS).approve(address(ynlsd), type(uint256).max);
ynlsd.initialize(init);

vm.prank(actors.STAKING_ADMIN);
Expand Down
3 changes: 2 additions & 1 deletion test/foundry/integration/Upgrades.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,8 @@ contract UpgradesTest is IntegrationBaseTest {
lsdRestakingManager: actors.LSD_RESTAKING_MANAGER,
lsdStakingNodeCreatorRole: actors.STAKING_NODE_CREATOR,
pauseWhitelist: pauseWhitelist,
pauser: actors.PAUSE_ADMIN
pauser: actors.PAUSE_ADMIN,
depositBootstrapper: actors.DEPOSIT_BOOTSTRAPER
});

return (init, ynlsd);
Expand Down
92 changes: 81 additions & 11 deletions test/foundry/integration/ynLSD.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import {TestLSDStakingNodeV2} from "test/foundry/mocks/TestLSDStakingNodeV2.sol"
import {TestYnLSDV2} from "test/foundry/mocks/TestYnLSDV2.sol";
import {ynBase} from "src/ynBase.sol";


contract ynLSDAssetTest is IntegrationBaseTest {
function testDepositSTETHFailingWhenStrategyIsPaused() public {
IERC20 asset = IERC20(chainAddresses.lsd.STETH_ADDRESS);
Expand All @@ -38,7 +37,7 @@ contract ynLSDAssetTest is IntegrationBaseTest {
assets[0] = asset;
amounts[0] = amount;

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

uint256 initialSupply = ynlsd.totalSupply();
uint256 initialTotalAssets = ynlsd.totalAssets();

// Obtain STETH
(bool success, ) = chainAddresses.lsd.STETH_ADDRESS.call{value: amount + 1}("");
require(success, "ETH transfer failed");
Expand All @@ -58,15 +60,18 @@ contract ynLSDAssetTest is IntegrationBaseTest {
stETH.approve(address(ynlsd), 32 ether);
ynlsd.deposit(stETH, depositAmount, address(this));

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

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

uint256 initialSupply = ynlsd.totalSupply();
uint256 initialTotalAssets = ynlsd.totalAssets();

// Obtain STETH
(bool success, ) = chainAddresses.lsd.STETH_ADDRESS.call{value: amount + 1}("");
require(success, "ETH transfer failed");
Expand All @@ -84,8 +89,8 @@ contract ynLSDAssetTest is IntegrationBaseTest {

uint256 totalDeposit = depositAmountOne + depositAmountTwo + depositAmountThree;

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

Expand Down Expand Up @@ -114,17 +119,29 @@ contract ynLSDAssetTest is IntegrationBaseTest {
ynlsd.convertToShares(asset, amount);
}

function testConvertToSharesBootstrapStrategy() public {
vm.prank(actors.STAKING_NODE_CREATOR);
ynlsd.createLSDStakingNode();
uint256[] memory totalAssets = ynlsd.getTotalAssets();
ynlsd.nodes(0);

uint256 bootstrapAmountUnits = ynlsd.BOOTSTRAP_AMOUNT_UNITS() * 1e18 - 1;
assertEq(totalAssets[0], bootstrapAmountUnits, "Total assets should be equal to bootstrap amount");
}

function testConvertToSharesZeroStrategy() public {
vm.prank(actors.STAKING_NODE_CREATOR);
ynlsd.createLSDStakingNode();
uint256[] memory totalAssets = ynlsd.getTotalAssets();
ynlsd.nodes(0);
assertEq(totalAssets[0], 0, "Total assets should be zero");

assertEq(totalAssets[1], 0, "Total assets should be equal to bootstrap 0");
}

function testGetTotalAssets() public {
uint256 totalAssetsInETH = ynlsd.convertToETH(ynlsd.assets(0), ynlsd.BOOTSTRAP_AMOUNT_UNITS() * 1e18 - 1);
uint256 totalAssets = ynlsd.totalAssets();
assertEq(totalAssets, 0, "Total assets should be zero");
assertEq(totalAssets, totalAssetsInETH, "Total assets should be equal to bootstrap amount converted to its ETH value");
}

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

assertEq(ynlsd.totalAssets(), 0);
assertEq(ynlsd.totalSupply(), 0);
// assertEq(ynlsd.totalAssets(), 0);
// assertEq(ynlsd.totalSupply(), 0);

assertEq(timeStamp > 0, true, "Zero timestamp");
assertEq(price > 0, true, "Zero price");
Expand All @@ -164,6 +181,8 @@ contract ynLSDAssetTest is IntegrationBaseTest {
vm.startPrank(unpauser);
pausableStrategyManager.unpause(0);
vm.stopPrank();

uint256 totalAssetsBeforeDeposit = ynlsd.totalAssets();

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


{
IERC20[] memory assets = new IERC20[](1);
uint256[] memory amounts = new uint256[](1);
Expand All @@ -194,7 +214,7 @@ contract ynLSDAssetTest is IntegrationBaseTest {
uint256 expectedBalance = balanceInStrategyForNode * oraclePrice / 1e18;

// Assert that totalAssets reflects the deposit
assertEq(totalAssetsAfterDeposit, expectedBalance, "Total assets do not reflect the deposit");
assertEq(totalAssetsAfterDeposit - totalAssetsBeforeDeposit, expectedBalance, "Total assets do not reflect the deposit");
}

function testPreviewDeposit() public {
Expand Down Expand Up @@ -527,4 +547,54 @@ contract ynLSDTransferPauseTest is IntegrationBaseTest {
assertFalse(isFirstAddressWhitelisted, "First new whitelist address was not removed");
assertFalse(isSecondAddressWhitelisted, "Second new whitelist address was not removed");
}
}


contract ynLSDDonationsTest is IntegrationBaseTest {

function testYnLSDdonationToZeroShareAttackResistance() public {

uint INITIAL_AMOUNT = 10_000 ether;

address _alice = makeAddr("Alice");
address _bob = makeAddr("Bob");

IERC20 assetToken = IERC20(chainAddresses.lsd.STETH_ADDRESS);

vm.deal(_alice, INITIAL_AMOUNT);
vm.deal(_bob, INITIAL_AMOUNT);


IERC20 steth = IERC20(chainAddresses.lsd.STETH_ADDRESS);

// get stETH
vm.startPrank(_alice);
(bool success, ) = chainAddresses.lsd.STETH_ADDRESS.call{value: INITIAL_AMOUNT}("");
require(success, "ETH transfer failed");

steth.approve(address(ynlsd), type(uint256).max);

vm.startPrank(_bob);
(success, ) = chainAddresses.lsd.STETH_ADDRESS.call{value: INITIAL_AMOUNT}("");
require(success, "ETH transfer failed");

steth.approve(address(ynlsd), type(uint256).max);

// Front-running part
uint256 bobDepositAmount = INITIAL_AMOUNT / 2;
// Alice knows that Bob is about to deposit INITIAL_AMOUNT*0.5 ATK to the Vault by observing the mempool
vm.startPrank(_alice);
uint256 aliceDepositAmount = 1;
uint256 aliceShares = ynlsd.deposit(assetToken, aliceDepositAmount, _alice);
assertEq(aliceShares, 0); // Since there are boostrap funds, this has no effect
// Try to inflate shares value
assetToken.transfer(address(ynlsd), bobDepositAmount);
vm.stopPrank();

// Check that Bob did not get 0 share when he deposits
vm.prank(_bob);
uint256 bobShares = ynlsd.deposit(assetToken, bobDepositAmount, _bob);

assertGt(bobShares, 1 wei, "Bob's shares should be greater than 1 wei");
}
}

0 comments on commit 7fd8daf

Please sign in to comment.