Tadle

Tadle
DeFiFoundry
27,750 USDC
View results
Submission Details
Severity: low
Invalid

Tadle::All Contracts - Incorrect Usage of Non-Upgradeable Ownable and Pausable Libraries

Summary

The upgradeable contracts in the system, including TokenManger.sol, are using non-upgradeable versions of OpenZeppelin's Ownable and Pausable libraries. This can lead to malfunctioning access control and pausability in proxy-based upgradeable contracts.

Vulnerability Details

The contracts are designed to be deployed as upgradeable proxy contracts, but they're using non-upgradeable versions of OpenZeppelin libraries:

import "@openzeppelin/contracts/access/Ownable.sol";
import "@openzeppelin/contracts/security/Pausable.sol";

Instead of:

import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";

exploit senario :

  1. Non-upgradeable Ownable sets the owner in the constructor, which isn't called in proxy deployments.

  2. Proxy-based upgradeability doesn't allow constructors, leading to no owner being set.

  3. All onlyOwner functions become inaccessible as there's no owner.

  4. Similar issues apply to the Pausable functionality.

Impact

Using the incorrect version of the Ownable library can result in a complete malfunction of all onlyOwner functions. Without ownership, critical administrative functions could be permanently inaccessible like updateTokenWhiteListed, initialize,SystemConfig.sol::All Function . and no address has the ability to pause the contract in case of emergencies.

Tools Used

Manual code Review

Recommendations

Replace non-upgradeable libraries with their upgradeable counterparts:

import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";
Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

[invalid] finding-Rescuable-upgradeable-initializeOwner

Valid high severity, since `initializeOwner` is not called for proxy contracts and the constructor for each `Rescuable.sol` contract will not be invoked during proxy deployment, this leaves the `owner` for each proxy unitialized allowing potential to withdraw fund from other proxy contracts inheriting `Rescuable.sol` respectively.

Appeal created

0xnevi Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

[invalid] finding-Rescuable-upgradeable-initializeOwner

Valid high severity, since `initializeOwner` is not called for proxy contracts and the constructor for each `Rescuable.sol` contract will not be invoked during proxy deployment, this leaves the `owner` for each proxy unitialized allowing potential to withdraw fund from other proxy contracts inheriting `Rescuable.sol` respectively.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.