Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sparkly Ruby Rabbit - Compromise of contractAddressManager #136

Open
sherlock-admin4 opened this issue Dec 30, 2024 · 0 comments
Open

Sparkly Ruby Rabbit - Compromise of contractAddressManager #136

sherlock-admin4 opened this issue Dec 30, 2024 · 0 comments

Comments

@sherlock-admin4
Copy link
Contributor

Sparkly Ruby Rabbit

High

Compromise of contractAddressManager

Summary

A malicious or compromised admin (or someone controlling the registry keys) for contractAddressManager will forcibly graduate markets and drain marketFunds belonging to all users. This amounts to total asset theft once the manager is compromised.

ReputationMarket fully trusts contractAddressManager to return the authorized "GRADUATION_WITHDRAWAL" address. If an attacker can change that registry entry—even briefly—they gain the power to:

  • Call graduateMarket(profileId), thus locking out new trades on that market.
  • Immediately call withdrawGraduatedMarketFunds(profileId), funneling the entire marketFunds[profileId] balance into the attacker’s account.

No fallback or secondary check exists in ReputationMarket to prevent an unexpected registry update. Therefore, the moment contractAddressManager is compromised, all un-graduated funds are at risk.

Why It Matters?

This is not a hypothetical edge case—key or registry compromises happen routinely in DeFi via phishing, wallet malware, social engineering, or direct contract exploit. The entire security of user funds depends on the single external contract address manager not being compromised.

Root Cause

In ReputationMarket.sol:

function graduateMarket(uint256 profileId) public whenNotPaused activeMarket(profileId) nonReentrant {
  address authorizedAddress = contractAddressManager.getContractAddressForName("GRADUATION_WITHDRAWAL");
  if (msg.sender != authorizedAddress) revert UnauthorizedGraduation();

  // ... mark market graduated
}

function withdrawGraduatedMarketFunds(uint256 profileId) public whenNotPaused nonReentrant {
  address authorizedAddress = contractAddressManager.getContractAddressForName("GRADUATION_WITHDRAWAL");
  if (msg.sender != authorizedAddress) revert UnauthorizedWithdrawal();

  // transfer marketFunds[profileId] to msg.sender
}
  • No internal reference to a stable or hardcoded “graduation contract”—the only check is msg.sender == addressGivenByManager.
  • If contractAddressManager can be changed (or social-engineered, or its private keys stolen), the attacker sets -- "GRADUATION_WITHDRAWAL" → attackerAddress.
  • That’s all it takes. There are no timelocks, multi-sig checks, or alerts stopping an immediate drain.

Internal Pre-conditions

1-contractAddressManager.getContractAddressForName("GRADUATION_WITHDRAWAL") is the only check for the graduation/withdraw logic.
2-There is no second internal gating or event-based time delay in ReputationMarket to verify changes in that address.

External Pre-conditions

1-The attacker gains enough access to contractAddressManager to overwrite the "GRADUATION_WITHDRAWAL" entry. This can happen through:

  • Key compromise: A dev or admin’s private key stolen via phishing, wallet exploit, etc.
  • Registry hack: The manager is itself an upgradeable or externally governed contract, subject to an exploit.
  • Social engineering: Trick the rightful manager admin into pointing "GRADUATION_WITHDRAWAL" to a malicious address.

2-The protocol has large marketFunds[...] accumulated from real users trading trust/distrust votes.

Attack Path

1-Attacker obtains control of the contractAddressManager. This could be instantaneous (stealing a key) or gradual (exploiting a bug in the registry contract).

2-They call (for example):
contractAddressManager.setContractAddressForName("GRADUATION_WITHDRAWAL", attackerEOA);

3-Attacker from attackerEOA (which is now recognized as the authorized graduation contract) calls:

reputationMarket.graduateMarket(profileId);
This flips the market to “graduated,” halting any further trading.

4-Immediately calls:
reputationMarket.withdrawGraduatedMarketFunds(profileId);
transferring all marketFunds[profileId] to attackerEOA.

5-Repeats for every active market. The attacker drains all user-deposited ETH, leaving the contract with zero balance.

Impact

Affected Party: Every user or participant who has deposited ETH into active (non-graduated) markets.
Loss:

  • Complete: The attacker can systematically remove all funds from every single market.
  • Irrecoverable: The protocol has no built-in safeguard or fallback once the attacker is recognized as “graduation.”

Scenario:
If the contract is popular, it might hold thousands of ETH. A single “manager key” slip or an unpatched bug in the manager contract—poof, all user funds gone.

PoC

// Hypothetical scenario demonstrating an attacker controlling the manager:

// 1. Attacker or compromised manager sets the GRADUATION_WITHDRAWAL address to themselves:
contractAddressManager.setContractAddressForName("GRADUATION_WITHDRAWAL", attackerAddress);

// 2. Now from 'attackerAddress':
reputationMarket.graduateMarket(profileId);    // Market is marked graduated
reputationMarket.withdrawGraduatedMarketFunds(profileId); 
// => Attacker receives all ETH locked in marketFunds[profileId]

(Note: The actual code depends on how contractAddressManager is implemented, but the vulnerability is the same.)

Why This Could Happen ( Examples)

Private Key Exploit:
Admin’s MetaMask is compromised. Attacker pushes a single transaction changing the registry.

Registry Contract Vulnerability:
A missing onlyOwner check or a flawed upgrade path lets an attacker hijack the manager.

Social Engineering:
Attacker convinces an admin that “the new graduation contract address is X,” admin calls setContractAddressForName("GRADUATION_WITHDRAWAL", X), not realizing X is a malicious EOA.

Rug Pull:
A malicious insider or original developer uses the manager to rug the protocol once enough user funds accumulate.

Mitigation

Strong Security on Manager:
The registry (or any method to update "GRADUATION_WITHDRAWAL") must be behind a multi-sig or DAO governance with time delays. This ensures no single key can instantly update addresses.

Immutable Graduation Address:
Hardcode the valid GRADUATION_WITHDRAWAL into the contract, removing the registry step. This is the strongest approach but sacrifices upgradability or flexibility.

Emergency Pause:
Possibly provide a secondary check that if "GRADUATION_WITHDRAWAL" changes unexpectedly, the protocol enters a paused or limited mode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant