Does not emit event after writing into storage

[hats-bug-reporter[bot]]

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

Description: Description

SafeWebAuthnSharedSigner::configue sets Sets the signer configuration for the calling account but the issue it doesn't emit evet after writing into storage.

    function configure(Signer memory signer) external onlyDelegateCall {
        uint256 signerSlot = SIGNER_SLOT;
        Signer storage signerStorage;

        // solhint-disable-next-line no-inline-assembly
        assembly ("memory-safe") {
            signerStorage.slot := signerSlot
        }

        signerStorage.x = signer.x;
        signerStorage.y = signer.y;
        signerStorage.verifiers = signer.verifiers;
    }

emit the event after writing into storage

[nlordell]

While I do think this is a reasonable suggestion, we wanted to make sure to keep Safe deployment gas costs (the point in which configure will be called) to a minimum. As such, we intentionally did not emit an event.

I'm also not sure that this falls under the "low" description as:

Issues where the behavior of the contracts differs from the intended behavior (as described in the docs and by common sense), but no funds are at risk.

An event being emitted was not documented, and in our opinion is not part of the expected behaviour of this function.

[0xEricTee]

@nlordell Event emissions offer transparency and on chain information about state changes of x, y and verifiers values.

Similar issue has been awarded Low severity in previous audit contests in Hats Finance. However, sponsor has the final decision to decide the validity of the issues. If this is intentional design, then I believe this issue would be invalid, thanks.

[nlordell]

We haven't made the final decision yet as we are evaluating internally (hence, I left the question label on the issue).

Thanks for the additional context @0xEricTee - this is helpful.

[nlordell]

We have decided to implement the suggestion and assign a low finding to this.

[nlordell]

A fix was implemented here: https://github.com/hats-finance/Safe-0x2909fdefd24a1ced675cb1444918fa766d76bdac/pull/23

Please let me know if this addresses the reported issue :). Thanks again for your submission, and @0xEricTee for participating in the discussions.

[0xEricTee]

A fix was implemented here: #23

Please let me know if this addresses the reported issue :). Thanks again for your submission, and @0xEricTee for participating in the discussions.

@nlordell Fix looks good, thanks!