Public functions not called by the contract should be declared external instead

Contracts in Solidity have the ability to override functions from their parent contracts and change the visibility of those functions. One common practice is to change the visibility from external to public. However, doing so can introduce potential security vulnerabilities.

The difference between external and public functions

In Solidity, external functions can only be called from other contracts, while public functions can be called both externally and internally.

The problem of using public for unused functions

Contracts typically have some functions that are meant to be called externally, and others that are only meant to be called internally. By default, if a function is not called by the contract itself, it is declared as external. However, contracts can override this behavior by declaring the function as public instead.

The issue arises when a public function is not called by the contract itself, but it is still accessible externally. This can lead to unintended usage of the function, potentially allowing attackers to exploit security vulnerabilities.

How to mitigate the problem

To ensure that unused functions are not accessible externally, it is recommended to declare them as external instead of public. By doing so, the unused functions can only be called from other contracts, preventing any accidental or malicious external access.

Here is an example to illustrate the difference:

contract Parent {
    function externalFunction() external {
        // This function can only be called by other contracts
    }
    
    function publicFunction() public {
        // This function can be called by both external callers and the contract itself
    }
}

contract Child is Parent {
    // Overrides the external function and changes visibility to public
    function externalFunction() public override {
        // Code implementation
    }
}

In this example, the externalFunction in the Child contract had its visibility changed to public. This means that the function can now be called externally, even though it was not intended to be. To prevent this unintended external access, the externalFunction should be declared as external in the Child contract as well.

Conclusion

When dealing with functions that are not meant to be called externally, it is essential to declare them as external instead of public to mitigate potential security risks. By following this best practice, you can ensure that your contracts remain secure and only allow the intended access.

Remember, always consider the visibility of your functions and be cautious when overriding parent functions. Stay vigilant in understanding and mitigating potential security vulnerabilities.