Skip to content

Latest commit

 

History

History
71 lines (61 loc) · 3.29 KB

File metadata and controls

71 lines (61 loc) · 3.29 KB

Rhythmic Azure Manatee

Medium

The bulkProcessPendingYield function is external and callable by any account

The bulkProcessPendingYield function is external and callable by any account

https://github.com/sherlock-audit/2025-01-peapods-finance/blob/main/contracts/contracts/BulkPodYieldProcess.sol#L20-L26

Description

The bulkProcessPendingYield function is external and callable by any account. This design choice can be deliberate (e.g., a public keeper pattern), but it also opens the possibility of repeated or spam calls. An actor—malicious or not—could invoke this function at will, potentially causing operational overhead or consuming block gas limits.

Impact (DoS / Spamming Concern)

  • Spam Calls: Repeated invocations by a malicious actor might bloat transaction history and cause elevated gas costs for legitimate users or keepers.
  • DoS Vector: If the underlying ITokenRewards logic is gas-intensive or triggers state updates that can fail, repeated spamming could temporarily block or disrupt legitimate yield-harvesting calls.

Proof of Concept

  1. Setup: Deploy BulkPodYieldProcess and a mock IDecentralizedIndex and ITokenRewards contract.
  2. Repeated Calls:
    // Attacker script, pseudo-code:
    for (uint i = 0; i < 100; i++) {
        bulkPodYieldProcess.bulkProcessPendingYield(indexArray);
    }
  3. Result:
    • The contract accepts each call (no restriction).
    • If the yield-processing function is gas-intensive, repeated calls might cause short-term DoS for other functions on the network or consume substantial gas from any keepers trying to do a genuine harvest.

Foundry Test Append this foundry test below to the following file and run forge test: contracts/test/BulkPodYieldProcess.t.sol.

// function is called by anyone
function testBulkProcessPendingYieldByAnyone() public {
        IDecentralizedIndex[] memory idxArray = new IDecentralizedIndex[](indices.length);
        for (uint256 i = 0; i < indices.length; i++) {
            idxArray[i] = indices[i];
        }
        vm.startPrank(address(0xfeefdeef));
        processor.bulkProcessPendingYield(idxArray);
        vm.stopPrank();
        // Verify each index's staking pool and rewards were accessed correctly
        for (uint256 i = 0; i < indices.length; i++) {
            assertEq(indices[i].lpStakingPool(), stakingPools[i]);
            assertEq(MockStakingPoolToken(stakingPools[i]).POOL_REWARDS(), poolRewards[i]);
        }
    }

Log Results Of Foudry Test

# Test function is called by anyone
forge test --match-test testBulkProcessPendingYieldByAnyone
Ran 1 test for test/BulkPodYieldProcess.t.sol:BulkPodYieldProcessTest
[PASS] testBulkProcessPendingYieldByAnyone() (gas: 98677)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.12ms (437.58µs CPU time)

Ran 1 test suite in 144.57ms (2.12ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommendation

Restrict Access:

  • Use a role-based system (e.g., OpenZeppelin AccessControl) or simple ownership:
    import "@openzeppelin/contracts/access/Ownable.sol";
    
    contract BulkPodYieldProcess is Ownable {
      function bulkProcessPendingYield(IDecentralizedIndex[] memory _idx) external onlyOwner {
        // ...
      }
    }