code-423n4 / 2023-06-stader-findings

1 stars 1 forks source link

The utilization of a hardcoded time value is incorrect when deployed to blockchains other than Ethereum #395

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderStakePoolsManager.sol#L57 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderOracle.sol#L26 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderOracle.sol#L70-L73 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/Auction.sol#L22

Vulnerability details

Medium

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderStakePoolsManager.sol#L57 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderOracle.sol#L26 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderOracle.sol#L70-L73 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/Auction.sol#L22

Title: The utilization of a hardcoded time value is incorrect when deployed to blockchains other than Ethereum

Impact

The hardcoded value of the MIN_AUCTION_DURATION constant in the Auction contract becomes problematic
when deploying the contracts on faster blockchains like Polygon and Binance Smart Chain.
Due to the shorter block times on these blockchains, the intended 24-hour auction duration is reduced significantly.

For instance, Binance Smart Chain's 3-second block time is a quarter of Ethereum's block time.
Consequently, the MIN_AUCTION_DURATION value of 7200, meant to represent 24 hours on Ethereum,
translates to only around 6.5 hours on Binance Smart Chain.

This discrepancy in auction durations across blockchains opens up arbitrage opportunities, as different auction lengths affect the outcomes.

The same hardcoded value is used inside Auction, StaderOrcale and StaderStakePoolsManager.

Inside StaderStakePoolsManager, the presence of a shorter deposit cooldown period introduces a vulnerability
that allows users to deposit significantly more funds into a pool than originally intended.

Proof of Concept

Stader should be deployed on different chains. (ref. https://blog.staderlabs.com/ethx-deposits-bda0f62d8ed8)

Auction uses hardcoded time for MIN_AUCTION_DURATION, and calculates the auction duration with this value.

uint256 public constant MIN_AUCTION_DURATION = 7200; // 24 hours
duration = 2 * MIN_AUCTION_DURATION;

The intended min duration of 24h will not apply on other chains then ethereum.

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/Auction.sol#L38 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/Auction.sol#L22

StaderStakePoolsManager initializes the contract with excessETHDepositCoolDown = 3 * 7200.

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderStakePoolsManager.sol#L57

StaderOracle set the max er update frequency to 7 days. This will only work for ethereum.

uint256 public constant MAX_ER_UPDATE_FREQUENCY = 7200 * 7; // 7 days

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderOracle.sol#L26 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderOracle.sol#L70-L73 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/Auction.sol#L22

Recommended Mitigation Steps

Add a variable "blocksInDay", which can be set inside the constructor.


uint256 blocksInDay = 7200; //use 7200 for ethereum mainnet

constructor(_blocksInDay){
  blocksInDay = _blocksInDay;
}

Assessed type

Timing

c4-judge commented 1 year ago

Picodes marked the issue as primary issue

manoj9april commented 1 year ago

ETHx smart contracts are for Ethereum blockchain alone. The code is not going to be ported over to any other chain.

c4-sponsor commented 1 year ago

manoj9april marked the issue as sponsor disputed

c4-judge commented 1 year ago

Picodes marked the issue as unsatisfactory: Invalid