c4-submissions commented 10 months ago

Lines of code

https://github.com/open-dollar/od-contracts/blob/v1.5.5-audit/src/contracts/proxies/Vault721.sol#L77

Vulnerability details

Impact

The ability to freely deploy ODProxy contracts through the Vault721 contract's build function represents a significant security vulnerability. Exploitation of this vulnerability could lead to:

Unauthorized Actions: Malicious actors could deploy proxies for unsuspecting users, allowing them to perform actions on behalf of those users without their consent. This action breaches the access control mechanisms intended by the system.
Asset Misappropriation: With control over a user's proxy, an attacker could potentially access, manipulate, or transfer the victim's assets or funds stored within the system.
Value Distortion: The potential exists for debt to be minted against an NFV in the same transaction as its transfer. This capability could mislead potential buyers regarding the actual value of an NFV.
System Trust Erosion: Such a vulnerability undermines the fundamental guarantees the system aims to provide, like ensuring that only NFV owners can mint debt against it. This flaw could lead to a loss of user trust and confidence in the platform's security and reliability.

In essence, the vulnerability poses significant financial, operational, and reputational risks to the platform and its users.

Proof of Concept

Code References:

Vault721 build function: This link points to the specific line in the Vault721 contract where the build function is defined, highlighting the absence of appropriate checks.
ODProxy Contract: This link provides a reference to the entire ODProxy contract, showing its minimalistic design and how it can be exploited if deployed maliciously.

  ///
  function build() external returns (address payable _proxy) {
    if (!_isNotProxy(msg.sender)) revert ProxyAlreadyExist();
    _proxy = _build(msg.sender);
  }

  /**
   * @dev allows user without an ODProxy to deploy a new ODProxy
   */
  function build(address _user) external returns (address payable _proxy) {
    if (!_isNotProxy(_user)) revert ProxyAlreadyExist();
    _proxy = _build(_user);
  }

Tools Used

VS code

Recommended Mitigation Steps

Restrict Proxy Deployment:
- Modify the build function in the Vault721 contract to include checks ensuring that only the rightful owner or authorized individuals can deploy an ODProxy.

function build() external returns (address payable _proxy) {
    // Check if the sender already has a proxy
    if (!_isNotProxy(msg.sender)) revert ProxyAlreadyExist();

    // Check if the sender is the owner of the NFV
    require(ownerOf(/*NFV_ID*/) == msg.sender, "Not the owner of the NFV");

    _proxy = _build(msg.sender);
}

function build(address _user) external returns (address payable _proxy) {
    // Check if the user already has a proxy
    if (!_isNotProxy(_user)) revert ProxyAlreadyExist();

    // Check if the sender is the owner of the NFV
    require(ownerOf(/*NFV_ID*/) == msg.sender, "Not the owner of the NFV");

    _proxy = _build(_user);
}

The placeholder /NFV_ID/ should be replaced with the appropriate mechanism to fetch the NFV ID related to the user. This assumes that ownerOf is a function provided by the ERC721 standard that gets the owner of a specific token ID. The check ensures that the sender is indeed the owner of the NFV before allowing them to deploy a proxy.

Enhance Access Control:
- Introduce role-based access control (RBAC) or other permission structures to manage who can execute specific functions, especially those related to proxy creation and asset management.
Audit SafeManager:
- Ensure that the SafeManager contract is resistant to reentrancy attacks and that it properly interacts with ODProxy and other contracts.
Introduce Rate Limiting:
- Implement rate limiting for sensitive functions, which can deter mass exploitation attempts and give developers more time to react to unusual behaviors.
Monitoring and Alerts:
- Implement real-time monitoring tools to detect and alert on suspicious activities, especially around proxy deployments and high-value transactions.
Emergency Shutdown Mechanism:
- Consider adding an emergency shutdown or pause mechanism that can halt certain functions in the event of a detected exploit.

By implementing these steps, the platform can significantly reduce the risks associated with the current vulnerability and bolster its overall security posture.