Project
One World
NFTDeFi
15,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

The `indexed` Keyword in Events Causes Data Loss for variables of type `string`

Fortis Audits

Summary

By making the event topic indexed for string type inside the MemberShipFactory contract, it would lead for wrong variable to be emitted

Vulnerability Details

  • 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 MemberShipFactory contract. The event MembershipDAONFTCreated is defined in such a way that the string variable of ensName is indexed.
The function createNewDAOMembership() is intended to create a new membership DAO NFT with the provided initialization DAO config and Tier config.
However, with the current design, the expected parameter wouldn't be emitted properly and front-end would get a meaningless one-way hash.

/// @notice Emitted when a new DAO NFT Membership is created, capturing the DAO configurations details and associated Tier configurations.
event MembershipDAONFTCreated(string indexed ensName, address nftAddress, DAOConfig daoData);

https://github.com/Cyfrin/2024-11-one-world/blob/1e872c7ab393c380010a507398d4b4caca1ae32b/contracts/dao/MembershipFactory.sol#L28

Impact

Proof of Concept

Consider this scenario as an example:

  1. The function createNewDAOMembership() is called

  2. Inside the function createNewDAOMembership() we expect to see the the string of ensName from calldata be emitted

  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 ensName would be lost in the DApp.

Proof of Code

  1. Create a BugTest.t.sol contract in the test folder.

  2. Add the following code to the BugTest.t.sol file:

  3. Run the test using the command forge test --mt test_FortisAudits_wrongEmit -vvvv.

// SPDX-License-Identifier: UNLICENSED
pragma solidity 0.8.22;
​
​
contract BugTest is Test {
​
function setUp() public {}
​
event MembershipDAONFTCreated_Indexed(string indexed ensName, address nftAddress);
event MembershipDAONFTCreated_NotIndex(string ensName, address nftAddress);
function test_FortisAudits_wrongEmit() public {
(DAOInputConfig memory config, TierConfig[] memory tiers) = Inputs();
emit MembershipDAONFTCreated_Indexed("DAO_test", address(0x123));
emit MembershipDAONFTCreated_NotIndex("DAO_test", address(0x123));
}
}

MembershipDAONFTCreated_Indexed event:

emit MembershipDAONFTCreated_Indexed(
ensName: 0x2004bda0238f6ce5d1c19118a52b956731365f186ff30dd2f469165b3b620b85,
nftAddress: 0x0000000000000000000000000000000000000123
)

MembershipDAONFTCreated_NotIndexed event:

emit MembershipDAONFTCreated_NotIndexed(
ensName: "DAO_test",
nftAddress: 0x0000000000000000000000000000000000000123
)

Mitigation

To mitigate this issue, remove the indexed keyword from the ensName parameter in the MembershipDAONFTCreated event.

Here is the recommended mitigation:

- event MembershipDAONFTCreated(string indexed ensName, address nftAddress, DAOConfig daoData);
+ event MembershipDAONFTCreated(string ensName, address nftAddress, DAOConfig daoData);
Updates

Lead Judging Commences

0xbrivan2 Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!