code-423n4 / 2022-12-gogopool-findings

1 stars 0 forks source link

Contract cannot be initialized due to revert #871

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/main/contracts/contract/tokens/TokenggAVAX.sol#L69-L79

Vulnerability details

Impact

TokenggAVAX.initialize() would revert due to the constructor setting initialized to type(uint8).max = 255 thus making _initialized not less than 1. This does not pass the require check in initializer modifier, thus resulting to a revert thereby making TokenggAVAX contract to never be initialized.

Proof of Concept

https://github.com/code-423n4/2022-12-gogopool/blob/main/contracts/contract/tokens/TokenggAVAX.sol#L69-L79

  1. TokenggAVAX.initialize() is deployed and calls _disableInitializers() which sets _initialized to 255.
  2. TokenggAVAX contract's initialize() function is called but reverts due to not passing the check for _initialized to be less than 1

Tools Used

Manual review

Recommended Mitigation Steps

_disableInitializers() can be avoided during the contract deployment and ensuring contract inititialize() function is callable only by contract owner or governor.

iFrostizz commented 1 year ago

This contract is not meant to be deployed directly, but to be called from a proxy, the constructor won't be executed. The function to disable initializers in the constructor was written to avoid this. Here is an helpful doc about the transparent proxy that has been used in the tests: https://docs.openzeppelin.com/contracts/3.x/api/proxy#TransparentUpgradeableProxy

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Insufficient quality

GalloDaSballo commented 1 year ago

Agree with the comment above