Tadle

Summary

As per solidity guidelines, the underscore prefix convention is suggested for non-external functions and state variables (private or internal). State variables without a specified visibility are internal by default.

Commit Hash: 04fd8634701697184a3f3a5558b41c109866e5f8

Repository URL: https://github.com/Cyfrin/2024-08-tadle/tree/main

Vulnerability Details

This convention is suggested for non-external functions and state variables (private or internal). State variables without a specified visibility are internal by default.

When designing a smart contract, the public-facing API (functions that can be called by any account) is an important consideration. Leading underscores allow you to immediately recognize the intent of such functions, but more importantly, if you change a function from non-external to external (including public) and rename it accordingly, this forces you to review every call site while renaming. This can be an important manual check against unintended external functions and a common source of security vulnerabilities (avoid find-replace-all tooling for this change).

Reference: https://docs.soliditylang.org/en/latest/style-guide.html#underscore-prefix-for-non-external-functions-and-variables

Impact

This is observed in the following functions

• Address::isContract

  • https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/libraries/Address.sol#L11C14-L11C24

• GenerateAddress::generateMakerAddress

  • https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/libraries/GenerateAddress.sol#L11C14-L11C34

• GenerateAddress::generateOfferAddress

  • https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/libraries/GenerateAddress.sol#L16

• GenerateAddress::generateStockAddress

  • https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/libraries/GenerateAddress.sol#L21C14-L21C34

• GenerateAddress::generateMarketPlaceAddress

  • https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/libraries/GenerateAddress.sol#L26

• RelatedContractLibraries::getCapitalPool

  • https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/libraries/RelatedContractLibraries.sol#L47C14-L47C28

• RelatedContractLibraries::getSystemConfig

  • https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/libraries/RelatedContractLibraries.sol#L26C14-L26C29

• RelatedContractLibraries::getPerMarkets

  • https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/libraries/RelatedContractLibraries.sol#L33C14-L33C27

• RelatedContractLibraries::getDeliveryPlace

  • https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/libraries/RelatedContractLibraries.sol#L40

• RelatedContractLibraries::getCapitalPool

  • https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/libraries/RelatedContractLibraries.sol#L47

• RelatedContractLibraries::getTokenManager

  • https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/libraries/RelatedContractLibraries.sol#L54C14-L54C29

Tools Used

• Manual Code Review: Analyzing the contract code directly.

• Static Analysis Tools: Slither - https://github.com/crytic/slither

Recommendations

• Refactor the functions to include an underscore prefix, to comply with the Solidity convention.