hats-finance / Inverter-Network-0xe47e52c4fea05e555920f1dcdcc6fb8eca103eeb

Fork of the Inverter Smart Contracts Repository
GNU Lesser General Public License v3.0
0 stars 3 forks source link

`LM_PC_KPIRewarder_v1.sol::postAssertion` lacks of Check-Effect-Interaction pattern usage #97

Open hats-bug-reporter[bot] opened 4 weeks ago

hats-bug-reporter[bot] commented 4 weeks ago

Github username: @erictee2802 Twitter username: 0xEricTee Submission hash (on-chain): 0x2ff66940a0f223abe645bfb15946bf08ceba2d1b8d1130541ba3eac6f9b41798 Severity: low

Description: Description\

During staking operation in LM_PC_KPIRewarder_v1.sol::postAssertion:

 function postAssertion(
        bytes32 dataId,
        uint assertedValue,
        address asserter,
        uint targetKPI
    ) public onlyModuleRole(ASSERTER_ROLE) returns (bytes32 assertionId) {

//REDACTED

        //--------------------------------------------------------------------------
        // Staking Queue Management

        for (uint i = 0; i < stakingQueue.length; i++) {
            address user = stakingQueue[i];
            _stake(user, stakingQueueAmounts[user]);
            totalQueuedFunds -= stakingQueueAmounts[user];
            stakingQueueAmounts[user] = 0;
        }

//REDACTED

The code above is lacking Check-Effect-Interaction pattern as the contract directly calls _stake function without first setting stakingQueueAmounts[user] to 0.

Attack Scenario\

Failing to follow CEI pattern can leave contracts vulnerable to reentrancy attacks.

Attachments

NA

  1. Proof of Concept (PoC) File

Refer above.

  1. Revised Code File (Optional)
 function postAssertion(
        bytes32 dataId,
        uint assertedValue,
        address asserter,
        uint targetKPI
    ) public onlyModuleRole(ASSERTER_ROLE) returns (bytes32 assertionId) {

//REDACTED

        //--------------------------------------------------------------------------
        // Staking Queue Management

        for (uint i = 0; i < stakingQueue.length; i++) {
            address user = stakingQueue[i];
++          uint stakingQueueAmounts = takingQueueAmounts[user];
++          stakingQueueAmounts[user] = 0;
++          _stake(user, stakingQueueAmounts);
++          totalQueuedFunds -= stakingQueueAmounts;
--          _stake(user, stakingQueueAmounts[user]);
--          totalQueuedFunds -= stakingQueueAmounts[user];
--          stakingQueueAmounts[user] = 0;
        }

//REDACTED
FHieser commented 2 weeks ago

PostAssertion() is controlled by a authorized and trusted Address which should prevent this