Reentrant Call Risk

[hats-bug-reporter[bot]]

Github username: @saidqayoumsadat Twitter username: saqsadat143 Submission hash (on-chain): 0x7b4e7ec3a5da298884c9471eb9410eb7496fbc3f35bf1f678897934863aee604 Severity: low

Description: Description\ The contract does not employ any reentrancy protection mechanisms. While reentrancy attacks are not directly apparent in the current implementation, it's a good practice to include checks-effects-interactions pattern or use the reentrancyGuard modifier to prevent unexpected behavior in future changes or in the contracts that interact with this one.

Attack Scenario

Description:

The contract lacks proper protection against reentrancy attacks. Reentrancy occurs when an external contract calls back into the vulnerable contract before the initial function call completes. In this contract, an attacker could potentially exploit this by creating a malicious contract that calls back into the create_pair function during its execution.

Scenario:

The attacker deploys a malicious contract that includes a reentrant function. The attacker sends a transaction to the create_pair function of the vulnerable contract, providing their malicious contract as one of the parameters. The create_pair function gets executed, and when the self._instantiate_pair function is called, control is transferred to the malicious contract. The malicious contract, during its execution, calls back into the vulnerable contract, potentially re-executing the create_pair function. This reentrant call allows the attacker to perform additional actions or manipulate the state of the vulnerable contract before the initial create_pair function completes.

Impact

The impact of a reentrancy attack could vary based on the logic and state changes within the create_pair function. It could potentially lead to unintended behavior, manipulation of state, or even financial losses depending on the contract's functionalities.

Attachments

1. Proof of Concept (PoC) File https://github.com/Cardinal-Cryptography/common-amm/blob/0a7264d707aea51b559a1bf94448681b59660f6a/amm/contracts/factory/lib.rs

2. Revised Code File (Optional)

use ink::{self, storage, Env};

trait ReentrancyGuard { fn try_enter(&mut self) -> bool; fn exit(&mut self); }

impl ReentrancyGuard for bool { fn try_enter(&mut self) -> bool { if self { false } else { self = true; true } }

fn exit(&mut self) {
    *self = false;
}

}

[ink(storage)]

struct FactoryContract { // existing state...

reentrancy_guard: bool,

}

impl FactoryContract { // existing functions...

fn _only_fee_setter(&self) -> Result<(), FactoryError> {
    if self.env().caller() != self.fee_to_setter || !self.reentrancy_guard.try_enter() {
        return Err(FactoryError::CallerIsNotFeeSetter);
    }
    Ok(())
}

This revised code includes a basic reentrancy guard using a boolean state variable `reentrancy_guard`. It is recommended to assess the entire contract structure for additional security considerations.

[deuszx]

Duplicate of https://github.com/hats-finance/AlephZeroAMM-0x0d88a9ece90994ecb3ba704730819d71c139f60f/issues/23#issuecomment-1902712937

[deuszx]

Thank you for participation. After carefully reviewing the submission we've concluded that this issue is INVALID.

We hope you participate in the future audits of ink!.