Thread by @milotruck

Wanted to share a vulnerability I've seen recently in multiple audit contests.

This could be your next high/medium severity finding on @code4rena or @sherlockdefi!

The vulnerability - using the "delete" keyword on @OpenZeppelin's EnumerableSet or EnumerableMap:

According to the following warning, an EnumerableSet cannot be cleared by simply using the "delete" keyword. Instead, this would corrupt its data:

github.com/OpenZeppelin/o…

Why is this so?

Under the hood, the EnumerableSet data structure contains an `_indexes` mapping which tracks the positions of values in the `_values` array:

github.com/OpenZeppelin/o…

Likewise, EnumerableMap contains a `_values` mapping that is used to access values by their keys:

github.com/OpenZeppelin/o…

Since Solidity does not keep track of the keys in a mapping, mappings cannot be cleared using the `delete` keyword.

Therefore, when `delete` is used on an EnumerableSet or EnumerableMap, their underlying mappings will remain uncleared.

What impact does this have?

For EnumerableSet:
• `contains()` returns true for previously stored values
• `add()` behaves as if the set still contains values
• `remove()` reverts as the set's length is now 0

I've created a simple Foundry test to demonstrate this:

For EnumerableMap:
• `contains()` returns true for previous keys
• `get()` can still be used to retrieve previous values
• `set()` doesn't increase the map's length for previous keys
• `at()` only returns keys added after the deletion occurred

Well, how are we supposed to delete them?

The proper way to clear an EnumerableSet would be to iterate over all its values and delete them individually:

This approach also applies to EnumerableMap:

How can we learn from such findings?

For auditors - always check the underlying implementation of libraries used.

In the worst-case scenario, you end up with a better understanding of how the library works, which might come in handy for future audits.

For developers, always be aware of the limitations of the libraries used in your contracts (ie. what can/cannot be done).

Additionally, read through the code of the libraries you use. Developers will often include comments that warn against potentially dangerous operations.

And that's all from me!

If you would like to run the tests shown above, they can be found here:

gist.github.com/MiloTruck/887e…

Also, here's an example of this finding on @sherlockdefi:

github.com/sherlock-audit…

Hope this helps you find a high/medium finding in the future!