hats-finance / Metrom-0xfdfc6d4ac5807d7460da20a3a1c0c84ef2b9c5a2

Smart contracts for the Metrom project.
GNU General Public License v3.0
0 stars 0 forks source link

The emit in the `acceptOwnership` function omits key information. #18

Open hats-bug-reporter[bot] opened 6 months ago

hats-bug-reporter[bot] commented 6 months ago

Github username: -- Twitter username: ACai_sec Submission hash (on-chain): 0x6c27d1435f675c243697eec48c5ce44bbb004c7e339d6eb1958cf3e9f9043bf9 Severity: low

Description: Description
The vulnerability in the acceptOwnership function of the smart contract arises due to the omission of critical information in the emitted event upon ownership transfer. Specifically, the AcceptOwnership event does not log the address of the new owner. This omission reduces the transparency and traceability of ownership changes, which is crucial for auditing and verifying ownership transitions in a contract where ownership control is a significant aspect.

Attack Scenario
While this particular issue does not directly enable a conventional security attack like theft or unauthorized access, it significantly hampers the ability to audit and validate the transition of contract ownership. Without adequate logging, it becomes challenging to ascertain the legitimacy of ownership changes, potentially leading to disputes or confusion regarding the current legitimate owner, especially in scenarios involving multiple stakeholders or in the aftermath of a contentious ownership transfer.

Attachments Recommend to fix the code as follow

    function acceptOwnership() external override {
        if (msg.sender != pendingOwner) revert Forbidden();
        // AcceptOwnership(oldOwner, newOwner)
        emit AcceptOwnership(owner, pendingOwner);
        delete pendingOwner;
        owner = msg.sender;

    }
luzzif commented 5 months ago

Addressed in https://github.com/metrom-xyz/contracts/commit/671c7e4d55cf63276ded85fd4361cff23647bb75