Lodestar-Finance / lodestar-protocol

Houses the code for the Lodestar Finance DeFi protocol.
BSD 3-Clause "New" or "Revised" License
10 stars 7 forks source link

Non-Input validation can lead to no-owner contracts and have to re-deploy #21

Closed maarcweiss closed 1 year ago

maarcweiss commented 1 year ago

TITLE (Non-Input validation can lead to no-owner contracts and have to re-deploy)

There is no input validation so the address of msg.sender is not 0. Generally projects do leave un-initialized variables to start, but in this case would cause a re-deployment and loss of gas because the only way to update the owner is via the following function:

https://github.com/LodestarFinance/lodestar-protocol/blob/807a166179ca5be52039d39316844d2a420eabce/contracts/Oracle/SushiOracle.sol#L50

which is impossible if the owner is address(0).

Input validation has to be done in all constructors from all contracts. I just pointed this one out, but should be done in all constructors. Check the fix below

SEVERITY (either high or medium, see the rules)

Medium, not very likely, but can cost loss of gas if happens and re-deployments

A LINK TO THE GITHUB ISSUE

https://github.com/LodestarFinance/lodestar-protocol/blob/807a166179ca5be52039d39316844d2a420eabce/contracts/Oracle/SushiOracle.sol#L23-L27

SOLUTION

Add input validation:

 //BEFORE
    constructor(address tokenA_, address tokenB_, address poolContract_) {
    admin = msg.sender;
    tokenA = tokenA_;
    tokenB = tokenB_;
    poolContract = poolContract_;
   }

   //AFTER
   constructor(address tokenA_, address tokenB_, address poolContract_) {
    require(admin == msg.sender, "Wrong owner");
   require(tokenA_ != address(0), "Token A address cannot be zero");
   require(tokenB_ != address(0), "Token B address cannot be zero");
   require(poolContract_ != address(0), "Pool contract address cannot be zero");

admin = msg.sender;
tokenA = tokenA_;
tokenB = tokenB_;
poolContract = poolContract_;
 }