Flow

Summary

The constructor in the NoDelegateCall contract lacks explicit visibility, which may lead to misunderstandings about its intended use and can affect code maintainability.

Finding Description

The constructor of the NoDelegateCall contract is defined without explicit visibility, meaning it defaults to internal. While this may seem harmless, not specifying visibility can lead to confusion for developers inheriting from this contract. Developers may mistakenly assume the constructor is public, leading to unintended access issues or incorrect assumptions about the contract's usage.

In a scenario where a derived contract needs to be instantiated externally, the lack of clarity may prevent proper deployment and interaction with the contract. This could break the security guarantee of restricting instantiation to intended contexts, as it may mislead developers into thinking they can instantiate the contract when they cannot.

Vulnerability Details

• Type: Constructor Visibility

• Severity: Medium

• Affected Component: Contract Instantiation

Impact

The impact assessment is rated as medium because while the contract will still function correctly, the lack of explicit visibility can lead to:

• Confusion for developers and misuse of the contract in derived implementations.

• Potentially incorrect assumptions about the contract's intended usage, leading to design flaws in the larger system where this contract is used.

Proof of Concept

To illustrate the potential confusion, consider a developer attempting to inherit from NoDelegateCall and instantiate it directly, assuming it to be public. They might write:

If they assume NoDelegateCall is public, they may be unaware that the constructor is actually internal, leading to compilation errors and misunderstanding of how the contract should be structured.

Recommendations

To fix this issue, explicitly define the constructor's visibility to clarify its intended use. Here’s a code snippet showing the recommended change:

Suggested Code Fix

This change makes it clear that the constructor is intended for use only within derived contracts, improving readability and maintainability.

File Location

NoDelegateCall.sol