45 lines
1.6 KiB
Markdown
45 lines
1.6 KiB
Markdown
# solc-0002 — ContractLevelChecker findDuplicateDefinitions — CLEAN (CWE-407)
|
|
|
|
## File
|
|
`libsolidity/analysis/ContractLevelChecker.cpp` ~line 239
|
|
|
|
## Pattern reviewed
|
|
```cpp
|
|
std::set<size_t> reported;
|
|
for (size_t i = 0; i < overloads.size() && !reported.count(i); ++i)
|
|
```
|
|
|
|
## CWE-407 verdict: NOT a performance defect
|
|
|
|
`std::set<size_t>::count` is O(log N), making the loop O(N log N) — well within
|
|
acceptable complexity for the use case. N is bounded by the number of overloaded
|
|
definitions of a single name in a contract, which in practice is tiny (< 20).
|
|
No CWE-407 patch warranted.
|
|
|
|
## Separate logic concern (not CWE-407)
|
|
|
|
The `!reported.count(i)` loop guard has a latent correctness defect: if index `i`
|
|
was inserted into `reported` as a *duplicate* during an earlier outer iteration
|
|
(e.g., iteration i=0 found j=2 is a duplicate and inserted 2), then when the outer
|
|
loop reaches i=2 the guard fires and the loop exits early. This causes i=2 to be
|
|
silently skipped as a *primary* candidate, potentially missing further duplicates
|
|
whose first occurrence is at index 2.
|
|
|
|
This is a correctness/logic defect, not a CWE-407 algorithmic-complexity defect.
|
|
It should be tracked separately if it produces missed duplicate-definition errors
|
|
in practice.
|
|
|
|
## Recommendation
|
|
|
|
Change the loop guard to allow all `i` to be visited as primary candidates:
|
|
|
|
```cpp
|
|
for (size_t i = 0; i < overloads.size(); ++i)
|
|
{
|
|
if (reported.count(i)) continue; // already flagged as someone else's dup
|
|
...
|
|
}
|
|
```
|
|
|
|
This preserves the intent (skip reporting i if it was already reported as a
|
|
duplicate of an earlier declaration) while not prematurely terminating the loop.
|