Skip to content

Latest commit

 

History

History
56 lines (39 loc) · 2.93 KB

File metadata and controls

56 lines (39 loc) · 2.93 KB

Bubbly Porcelain Blackbird

High

withdrawDonations() does not check whether the address of a profileId is compromised, allows stealing the enitre donated amount

Summary

The withdrawDonations() missed the isAddressCompromised(msg.sender) check before withdrawing the donations amount, https://github.com/sherlock-audit/2024-11-ethos-network-ii/blob/57c02df7c56f0b18c681a89ebccc28c86c72d8d8/ethos/packages/contracts/contracts/ReputationMarket.sol#L570

Root Cause

Under a profileId, there can be multiple registered addresses including the main address. All these addresses are also maps back to the profileId also. If one of address get compromised, the main address can perform a EthosProfile.deleteAddress() call, this marks the isAddressCompromised[compromiseAddress] to true.

The issue here is that the established market under a profileId can still have a non-zero donationEscrow[address] balance for the compromised address, which can be steal simply by calling the withdrawDonations() function.

Attack Path

  1. Alice createMarket() under a profileId==1, the donationRecipient[1] sets to address(alice)
  2. Alice updateDonationRecipient() to address(alice2),
  3. Market performed well, she received some donations, however, the current donationRecipient[address(alice2)] get compromised,
  4. Immediate action taken by Alice(the main address), calls deleteAddress(alice2,true), marking the address(alice2) compromised there,

Note: To take action against compromised address, two steps required considering address(alice) is an EOA, 1). marks the address compromise to prevent any further damages to profile, 2). updateDonationRecipient on ReputationMarket to avoid giving away the collected donation

  1. As another crucial step, Alice tries changing the donationRecipient by making a updateDonationRecipient call on the ReputationMarket
  2. However, attacker saw changed compromised status for address(alice2), and swiftly proceed to the withdrawDonations() before the Alice call lends first.

The attacker managed to steal the collected donation funds without needing to frontrun any of the txn(since its 2 step process).

Impact

Donation funds can be steal by a compromised address, due to missing isAddressCompromised check before withdrawing.

Mitigation

  function withdrawDonations() public whenNotPaused returns (uint256) {
+   if (_ethosProfileContract.isAddressCompromised(msg.sender)) {
+         revert AddressCompromised(msg.sender); 
+   }
    uint256 amount = donationEscrow[msg.sender];
    if (amount == 0) {
      revert InsufficientFunds();
    }

    // Reset escrow balance before transfer to prevent reentrancy
    donationEscrow[msg.sender] = 0;

    // Transfer the funds
    (bool success, ) = msg.sender.call{ value: amount }("");
    if (!success) revert FeeTransferFailed("Donation withdrawal failed");

    emit DonationWithdrawn(msg.sender, amount);
    return amount;
  }