Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

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

matin

Summary

By making the event topic indexed for string type inside the SystemConfig contract, it would lead for wrong variables 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 SystemConfig contract. The events CreateMarketPlaceInfo and UpdateMarket are defined in such a way that the string variable of marketPlaceName in both is indexed.

The functions createMarketPlace() and updateMarket() are intended to create a new marketplace and update a market with the provided data respectively. However, with the current design, the expected parameter (marketPlaceName) wouldn't be emitted properly and front-end would get a meaningless one-way hash.

/// @dev Emit events when create marketPlace info
event CreateMarketPlaceInfo(
string indexed marketPlaceName,
address indexed marketPlaceAddress,
bool indexed fixedratio
);
​
/// @dev Emit events when update marketPlace info
event UpdateMarket(
string indexed marketPlaceName,
address indexed marketPlaceAddress,
address indexed tokenAddress,
uint256 tokenPerPoint,
uint256 tge,
uint256 settlementPeriod
);

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/interfaces/ISystemConfig.sol#L55
https://github.com/Cyfrin/2024-08-tadle/blob/main/src/interfaces/ISystemConfig.sol#L62

Impact

Wrong and meaningless event variables would be emitted in the front-end

Proof of Concept

Consider this scenario as an example:

  1. The function createMarketPlace() or updateMarket() is called

  2. Inside the mentioned functions we expect to see the the string of marketPlaceName 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 marketPlaceName would be lost in the DApp.

For test purposes, one can run this test file:

​
event CreateMarketPlaceInfo(
string indexed marketPlaceName,
address indexed marketPlaceAddress,
bool indexed fixedratio
);
​
event CreateMarketPlaceInfo1(
string marketPlaceName,
address indexed marketPlaceAddress,
bool indexed fixedratio
);
​
function test_emitter() public {
​
string memory sampleString = "Sample String";
​
emit CreateMarketPlaceInfo(sampleString, address(this), true);
emit CreateMarketPlaceInfo1(sampleString, address(this), true);
}

Outputs of test:

event CreateMarketPlaceInfo:

{
"from": "0x9d83e140330758a8fFD07F8Bd73e86ebcA8a5692",
"topic": "0xffe2ec09af17510d3ae1109924fc3e736b5b3a562de523cf3e2c31331bd72499",
"event": "CreateMarketPlaceInfo",
"args": {
"0": {
"_isIndexed": true,
"hash": "0xef9c332eb645015be3459b0a6f8030525e9981ffa21d3bd5c857415266a6e752"
},
"1": "0x9d83e140330758a8fFD07F8Bd73e86ebcA8a5692",
"2": true
}
}

event CreateMarketPlaceInfo1:

{
"from": "0x9d83e140330758a8fFD07F8Bd73e86ebcA8a5692",
"topic": "0x2b031f9e3c2360fb549e625f31073d59031f6a7ffff021aa70dbd877c049dc1c",
"event": "CreateMarketPlaceInfo1",
"args": {
"0": "Sample String",
"1": "0x9d83e140330758a8fFD07F8Bd73e86ebcA8a5692",
"2": true,
"marketPlaceName": "Sample String",
"marketPlaceAddress": "0x9d83e140330758a8fFD07F8Bd73e86ebcA8a5692",
"fixedratio": true
}
}

Tools Used

Manual

Recommendations

/// @dev Emit events when create marketPlace info
event CreateMarketPlaceInfo(
- string indexed marketPlaceName,
+ string marketPlaceName,
address indexed marketPlaceAddress,
bool indexed fixedratio
);
​
/// @dev Emit events when update marketPlace info
event UpdateMarket(
- string indexed marketPlaceName,
+ string marketPlaceName,
address indexed marketPlaceAddress,
address indexed tokenAddress,
uint256 tokenPerPoint,
uint256 tge,
uint256 settlementPeriod
);
Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

[invalid] finding-SystemConfig-Indexed-string-event

Invalid, known issue [Low-24](https://github.com/Cyfrin/2024-08-tadle/issues/1)

Support

FAQs

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

Give us feedback!