Tadle

Summary

Vulnerability Details

The contracts TokenManager, PreMarktes, DeliveryPlace, SystemConfig and CapitalPool are implementations which are meant to be interacted with via proxy. but all have constructor() Rescuable() {} which is Problematic because Rescuable() uses OpenZeppelin's Ownable which modifies address private _owner in its constructor. Therefore, at deployment, the owner is set on the implementation and not in the Proxy contract storage.

In addition to that, TokenManager and SystemConfig contracts both have the initialize function which has the onlyOwner modifier, which is from Openzepplin's Ownable contract, this means that calling the initialize function checks if msg.sender corresponds to address private _owner which is on the implementation.

Relevant Github links

Openzeppelin's Ownable https://github.com/OpenZeppelin/openzeppelin-contracts/blob/c304b6710b4b5fcf2a319ad28c36c49df6caef14/contracts/access/Ownable.sol#L21 and Rescuable https://github.com/Cyfrin/2024-08-tadle/blob/main/src/utils/Rescuable.sol

Impact

Potentially unexpected behavior in future upgrades.

Tools Used

Manual Review

Recommendations

Use the OpenZeppelin's Initializable hence the initializer modifier and upgradeable contracts like OwnableUpgradeable