`indexed` Keyword in Events Causes Data Loss for Dynamic Array Variables

[hats-bug-reporter[bot]]

Github username: @MatinR1 Twitter username: MatinRezaii1 Submission hash (on-chain): 0x15191bdce8a415162459395c3c243c5b5fc5ecc75be921124457b7c11bf6706a Severity: low

Description: Description\ when the indexed keyword is used for reference type variables such as dynamic arrays or strings, it will return the hash of the mentioned variables. Thus, the event which is supposed to inform all of the applications subscribed to its emitting transaction (e.g. front-end of the DApp, or the backend listeners to that event), would get a meaningless and obscure 32 bytes that correspond to keccak256 of an encoded dynamic array. This may cause some problems on the DApp side and even lead to data loss. For more information about the indexed events, check here:

(https://docs.soliditylang.org/en/v0.8.17/abi-spec.html?highlight=indexed#events)

The problem exists inside the Rebalancing contract. The event UpdatedTokens is defined in such a way that the dynamical array of newTokens is indexed. With doing so, the expected parameters wouldn't be emitted properly and front-end would get meaningless one-way hashes.

Attachments

1. Proof of Concept (PoC) File Consider this scenario as an example:

1 - The function updateTokens() is called by the asset manager

2 - Inside the function updateTokens() we expect to see the the array of "_newTokens" be emitted:

  function updateTokens(
    FunctionParameters.RebalanceIntent calldata rebalanceData
  ) external virtual nonReentrant onlyAssetManager {

    ...

    emit UpdatedTokens(_newTokens);
  }

3 - But as the event topic is defined as indexed we'll get an obscure 32-byte hash and listeners will not be notified properly. Thus, the array of recently updated tokens would be lost in the DApp.

For test purpose one can run this test file:

    event UpdatedTokens(address[] indexed newTokens);
    event UpdatedTokens1(address[] newTokens);

    function test_emitter() public {

        address[] memory _newTokens = new address[](3);
        _newTokens[0] = (address(45621));
        _newTokens[1] = (address(16280123));
        _newTokens[2] = (address(29378601));

        emit UpdatedTokens(_newTokens);
        emit UpdatedTokens1(_newTokens);
    }

Outputs of test:

UpdatedTokens event:

{
        "from": "0x05f4f2871A340c4Bd22B44aC2040Ff2794aFD2A4",
        "topic": "0x78c2af8266e5e5c122300f6c72cc717e475bab04684f24645166d5df4b1ad4e0",
        "event": "UpdatedTokens",
        "args": {
            "0": {
                "_isIndexed": true,
                "hash": "0xbbc5483a19795bbbbd8c781bd967dd63baa732eedd40c64d80f20c8c3b688023"
            }
        }
    }

UpdatedTokens1 event:

{
        "from": "0x05f4f2871A340c4Bd22B44aC2040Ff2794aFD2A4",
        "topic": "0x9a0c58d29d2578781e4e704d5ca934c7341d6e2b614808d6432f71802370d8b9",
        "event": "UpdatedTokens1",
        "args": {
            "0": [
                "0x000000000000000000000000000000000000B235",
                "0x0000000000000000000000000000000000f86A3B",
                "0x0000000000000000000000000000000001C04829"
            ],
            "newTokens": [
                "0x000000000000000000000000000000000000B235",
                "0x0000000000000000000000000000000000f86A3B",
                "0x0000000000000000000000000000000001C04829"
            ]
        }
    }

As you can see, the intended outputs wouldn't be emitted if the indexed keyword is used for newTokens (address[]).

2. Revised Code File (Optional) Consider modifying the event UpdatedTokens inside the contract Rebalancing:

-   event UpdatedTokens(address[] indexed newTokens);
+   event UpdatedTokens(address[] newTokens);

[MatinR1]

Also, the mentioned issue exists in these events:

1. TokenWhitelisted
2. TokensRemovedFromWhitelist
3. UserWhitelisted
4. UserRemovedFromWhitelist
5. TokensEnabled
6. TokensDisabled