The nonReentrant modifier should be first in a function declaration

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0x2c97cb37390a4e34aae81b537a8603fa89d5cdb55843f50405934cc4352c36a5 Severity: low

Description: Description

In Solidity, the nonReentrant modifier is essential for securing smart contracts against reentrancy attacks. It should be positioned first in a function declaration. This placement is critical because it ensures that the modifier's protection is applied before any other code or modifiers in the function. If placed later, other modifiers could potentially be executed in a reentrant manner before nonReentrant has a chance to lock the function, leaving the contract vulnerable. Thus, prioritizing nonReentrant as the first modifier is a best practice for robust smart contract security, effectively guarding against unintended reentries and related exploits.

Attachments

1. Proof of Concept (PoC) File

['216']

216:     function depositFeeFunds(uint amount)
217:         external
218:         onlyOrchestratorAdmin
219:         nonReentrant // <= FOUND
220:         validAmount(amount)
221:     {
222:         defaultCurrency.safeTransferFrom(_msgSender(), address(this), amount);
223:
224:         emit FeeFundsDeposited(address(defaultCurrency), amount);
225:     }

2. Revised Code File (Optional)

    function depositFeeFunds(uint amount)
        external
++   nonReentrant
        onlyOrchestratorAdmin
--     nonReentrant
        validAmount(amount)
    {
        defaultCurrency.safeTransferFrom(_msgSender(), address(this), amount);

        emit FeeFundsDeposited(address(defaultCurrency), amount);
    }

[PlamenTSV]

Informational

[0xmahdirostami]

thank you @PlamenTSV