initialized storage variables are set again in the initializer function

[code423n4]

Handle

sabtikw

Vulnerability details

Impact

storage variables are initialized in the contract and overwritten in the initializer function.

Proof of Concept

Auction.sol L#89 L#164 auctionLength
AuctionBurnReserveSkew.sol L#25 auctionAverageLookback
MaltDataLab.sol L#69 priceTarget

Tools Used

manual review

Recommended Mitigation Steps

remove initialization outside of initializer function

[GalloDaSballo]

This could be even worse @0xScotch , please note that for upgradeable contracts, default values set inline will default to 0 / false / 0x as the proxy you'll be using won't go through setting those default values

[GalloDaSballo]

@0xScotch after careful review, I believe you need to urgently change the initialize function to set the default values.
If you use any proxy, those values will be set to 0.
Because initialize doesn't set them again, this can be dramattic

[GalloDaSballo]

The warden didn't catch the higher severity finding so am leaving this as gas

[0xScotch]

Hey @GalloDaSballo sorry for the late reply. The contracts are not meant to be upgradeable, the use of initializable was a habitual one. We plan on completely removing initializable and just use contructors instead due to the contracts not being upgradeable.

[GalloDaSballo]

@0xScotch Thank you for the clarification
Because of this, I'll be re-evaluating some of the findings.

@0xScotch Please note that because you are not using a proxy for deploying, your contract initialize may get frontrun, switching to constructor solves as well as enabling the usage of immutable

[0xScotch]

Yes we are aware of the frontrunning issue too. We will absolutely be switching to constructors.