Update initializer modifier to prevent reentrancy during initialization

[code423n4]

Lines of code

https://github.com/aave/lens-protocol/blob/main/package.json#L74 https://github.com/code-423n4/2022-02-aave-lens/blob/main/contracts/upgradeability/VersionedInitializable.sol#L20-L21 https://github.com/aave/protocol-v2/blob/6a503eb0a897124d8b9d126c915ffdf3e88343a9/contracts/protocol/libraries/aave-upgradeability/VersionedInitializable.sol#L2 https://github.com/aave/protocol-v2/commits/6a503eb0a897124d8b9d126c915ffdf3e88343a9/contracts/protocol/libraries/aave-upgradeability/VersionedInitializable.sol https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3006

Vulnerability details

This is mainly a risk I feel should be brought to the sponsor's and the judge's attention. I can't fully exploit and remediate to this, but I'll try to be as clear as possible in every steps, as it might be a high risk finding.

The issue

The solution uses "@openzeppelin/contracts": "4.4.0" (https://github.com/aave/lens-protocol/blob/main/package.json#L74)

This version has a known high risk vulnerability on initializer: https://snyk.io/test/npm/@openzeppelin/contracts/4.4.0 . This finding was published and disclosed around December 15th 2021. Here's the pull request: https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3006

However, this solution doesn't use OpenZeppelin's Initializable: it instead implements its own VersionedInitializable, inspired from Aave's version: https://github.com/aave/protocol-v2/blob/6a503eb0a897124d8b9d126c915ffdf3e88343a9/contracts/protocol/libraries/aave-upgradeability/VersionedInitializable.sol#L2

But this Aave Initializable is still inspired from the old OpenZeppelin's Initializable, and the last commit is from January 27th 2021 (almost a year before the 4.4.0's vulnerability discovery) : https://github.com/aave/protocol-v2/commits/6a503eb0a897124d8b9d126c915ffdf3e88343a9/contracts/protocol/libraries/aave-upgradeability/VersionedInitializable.sol .

With these lack of up-to-date information / versions and commit in mind: I believe the same vulnerability discovered in OpenZeppelin's Initializable 2 months ago can be replicated here and should be paid attention to.

While this is certainly a potential finding found by searching, a more experienced security researcher is needed to fully prove with a POC that VersionedInitializable is indeed vulnerable.

[Zer0dot]

This is valid, but the scope is restricted to having a corrupt proxy admin, as during deployment initialization is done in the proxy constructor (and even if it wasn't, there is basically nothing at risk during initial deployment).

[0xleastwood]

While I appreciate the in depth finding, I don't see any external calls made in any of the initialize() functions. As such, this finding doesn't have a lot of validity but I think it is fair to include this in the warden's QA report.

[0xleastwood]

As far as I'm aware, contracts containing an initialize() function will be part of a proxy construction which doesn't suffer from the issue raised by the warden.

[0xleastwood]

Merging into #60 and marking as invalid.