DMDcoin / diamond-contracts-claiming

claiming contracts
0 stars 2 forks source link

[H-02] Signature Replay Attack Vulnerability in ClaimContract #49

Open softstackio opened 2 months ago

softstackio commented 2 months ago

Description: The claim function in the ClaimContract is vulnerable to signature replay attacks. The function uses a _postfix parameter in the signature verification process, but there's no mechanism to ensure this postfix is unique for each claim. This oversight allows an attacker to potentially reuse a valid signature to claim funds multiple times, especially if the contract is redeployed or used on different chains. Impact:

  1. Multiple Claims: An attacker could use a single valid signature to claim funds multiple times, potentially draining the contract of more funds than intended.
  2. Cross-Chain Vulnerability: If the same contract is deployed on multiple chains, a signature used on one chain could be replayed on another.
  3. Redeployment Risk: If the contract is ever redeployed (e.g., for upgrades), old signatures could become valid again, allowing for duplicate claims.

The vulnerability lies in the claim function:

unction claim(
   address payable _targetAdress,
   bytes memory _postfix,
   bytes32 _pubKeyX,
   bytes32 _pubKeyY,
   uint8 _v,
   bytes32 _r,
   bytes32 _s
) external {
   // ... (other checks)
   if (!claimMessageMatchesSignature(_targetAdress, _postfix, _pubKeyX,
_pubKeyY, _v, _r, _s))
       revert ClaimErrorSignatureMissmatch();
   // ... (claim processing)
}

The _postfix parameter is intended to add uniqueness to the signature, but there's no enforcement of its uniqueness or validation of its content.

Proof of Concept:

  1. Alice generates a valid signature for claiming her funds, using a generic postfix.
  2. Alice successfully claims her funds.
  3. The contract is redeployed due to an upgrade.
  4. An attacker obtains Alice's old signature and uses it to claim funds again, as the new contract has no record of the previous claim.

Recommendation: To mitigate this vulnerability, implement one or more of the following measures:

  1. Nonce System: a. Introduce a nonce for each address that increments with each claim. b. Include this nonce in the signed message. c. Verify and increment the nonce during the claim process.

    mapping(bytes20 => uint256) public nonces;
    function claim(..., uint256 nonce, ...) external {
    require(nonce == nonces[oldAddress], "Invalid nonce");
    // ... (existing claim logic)
    nonces[oldAddress]++;
    }
  2. Single-Use Signatures: a. Store a hash of used signatures. b. Check against this storage to prevent reuse.

    mapping(bytes32 => bool) public usedSignatures;
    function claim(...) external {
    bytes32 sigHash = keccak256(abi.encodePacked(_v, _r, _s));
    require(!usedSignatures[sigHash], "Signature already used");
    usedSignatures[sigHash] = true;
    // ... (existing claim logic)
    }
  3. Chain ID Inclusion: a. Include the chain ID in the signed message to prevent cross-chain replay attacks.

cryptonit commented 2 months ago

there is only one message to sign in the dmd v3 wallet

its not split into multiple parts

so on dmd wallet side he need to sign a text that contain the prefix + target v4 address + if needed a variable postfix

but its all combined in one message

and the prefix is different on mainnet launch then on beta testnet for exact that reason that it cant be reused

user cant replay signatures as u correct did dedect its a different message needed so it cant be replayed

SurfingNerd commented 2 months ago

This attack possibility has been addressed in the design of the the claiming contracts.

 Multiple Claims: An attacker could use a single valid signature to claim funds multiple times, potentially draining the contract of more funds than intended.

By design, every entitlement can only be claimed once and completely, therefore using the same signature multiple times, can not lead to double claiming

Cross-Chain Vulnerability: If the same contract is deployed on multiple chains, a signature used on one chain could be replayed on another.

To address cross chain replays, we have included a prefix that needs to be signed. This prefix will be different on every our chains, so deployments on alpha networks, public beta networks and the final network use different prefixes to prevent from cross chain replay attacks.

Redeployment Risk: If the contract is ever redeployed (e.g., for upgrades), old signatures could become valid again, allowing for duplicate claims.

Thanks for pointing that out. Deploying, and filling this contract twice is a scenario that is not planned. Even if, it would follow the same logic than multiple claims ad cross chain vulnerability and a different prefix would be used.