Summary

The TadleFactory contract's deployUpgradeableProxy function creates new proxy contracts for each upgrade instead of upgrading existing proxies. While the system shows preparations for upgradability (such as storage gaps and inheritance from UpgradeableStorage), the deployUpgradeableProxy function creates new UpgradeableProxy instances each time it's called, overwriting the address in the relatedContracts mapping.

This approach doesn't leverage the upgradeable design of the contracts and can lead to state loss and system inconsistencies.

Vulnerability Details

Consider a scenario where an admin deploys a contract at index 1, and later, the same admin calls deployUpgradeableProxy with index 1 again, deploying a new contract. The previous contract's state is lost, and any functionality relying on that contract will fail.

In the TadleFactory::deployUpgradeableProxy :

This function creates a new UpgradeableProxy each time it's called, even for existing indices, leading to state loss and potential inconsistencies.

The UpgradeableProxy contract suggests that future variables will be added because of the __gap variable. The curent implementation will lead to a loss of the state of these future variables, as it won’t be possible to modify the deployUpgradeableProxy function.

Impact

• Loss of State: Overwriting existing proxies can lead to the loss of all state data held by the previous contract, disrupting the functionality of the system.

• Operational Disruption: If contracts are actively being used, replacing them can cause significant operational issues, leading to failures in dependent systems and processes.

• Inability to Upgrade Safely: The lack of a proper upgrade mechanism prevents the system from adapting to new requirements or fixing bugs without risking data loss or service interruptions.

Tools Used

Manual review

Recommendations

Implement a true upgrade mechanism:

• Modify the UpgradeableProxy contract to include functions for upgrading the logic contract without changing the proxy address (e.g., upgradeToAndCall).

• Add an upgradeProxy function to TadleFactory that calls the upgrade function on the existing proxy:

• Implement validation for the _relatedContractIndex parameter to prevent overwriting existing proxies without checks.

• Modify deployUpgradeableProxy to only deploy a new proxy if one doesn't exist for the given index. If a proxy already exists, it should revert or call the upgrade function instead, ensuring that the state is preserved.

• Consider using the upgrade function upgradeToAndCall provided by TransparentUpgradeableProxy effectively, in order to change the implementation.