Incorrect struct member order while emitting event

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x67941fe1f23e32d43ca3eab643a9006e2f5f41866fded4458d95bc0dbca2d76e Severity: minor

Description: Description\

In ink! based Most contract, there is an issue with incorrect event emission with respective struct member order.

Affected code:

https://github.com/hats-finance/Most--Aleph-Zero-Bridge-0xab7c1d45ae21e7133574746b2985c58e0ae2e61d/blob/70ab234cc3322fda82784413f5e0704907a0e1fe/azero/contracts/most/lib.rs#L412-L415

Reference code:

https://github.com/hats-finance/Most--Aleph-Zero-Bridge-0xab7c1d45ae21e7133574746b2985c58e0ae2e61d/blob/70ab234cc3322fda82784413f5e0704907a0e1fe/azero/contracts/most/lib.rs#L54-L61

In receive_request() function, An event is emitted when request signature count is incremented.

     . . . some code

            // record vote
            request.signature_count = request
                .signature_count
                .checked_add(1)
                .ok_or(MostError::Arithmetic)?;
            self.signatures.insert((request_hash, caller), &());

            self.env().emit_event(RequestSigned {   . . . . @audit // event struct members location altered
@>           signer: caller,
                request_hash,
            });

    . . . some code

RequestSigned event is emitted here, however the issue is, the struct member locations of RequestSigned does not match with the event. This can be checked below,

    #[ink(event)]
    #[derive(Debug)]
    #[cfg_attr(feature = "std", derive(Eq, PartialEq))]
    pub struct RequestSigned {
        pub request_hash: HashedRequest,
        #[ink(topic)]
        pub signer: AccountId,
    }

It can be seen, First request_hash and then signer is used as struct members, however while emitting events this is completely reversed.

POC\ When the user or reader see the RequestSigned event, the implementation of event will be different than the event is emitted. This will create confusion with code readers and when this info is stored then it will be catched and stored incorrectly as the both the struct member locations are reversed.

Recommendation to fix\

Ensure, the event struct members same while emiting events.

            // record vote
            request.signature_count = request
                .signature_count
                .checked_add(1)
                .ok_or(MostError::Arithmetic)?;
            self.signatures.insert((request_hash, caller), &());

            self.env().emit_event(RequestSigned {
-               signer: caller,
                request_hash,
+              signer: caller,
            });

[krzysztofziobro]

Not a vulnerability

[0xRizwan]

@krzysztofziobro @fbielejec

This is valid issue as the function is emitting wrong event as described in issue. Per the rules of contest, the issue is identified as Minor severity.

Attacks not mentioned in higher categories, but having a negative impact on user experience, or putting one of the contracts in an unexpected state.

[fbielejec]

not an issue