Skip to content

Latest commit

 

History

History
60 lines (49 loc) · 3.07 KB

006.md

File metadata and controls

60 lines (49 loc) · 3.07 KB

Rhythmic Azure Manatee

Medium

The VotingPool update function can be accessed by anyone to modify the internal staking state of the caller

The update function can be accessed by anyone to modify the internal staking state of the caller

https://github.com/sherlock-audit/2025-01-peapods-finance/blob/main/contracts/contracts/voting/VotingPool.sol#L58-L60

https://github.com/sherlock-audit/2025-01-peapods-finance/blob/main/contracts/contracts/voting/VotingPool.sol#L114-L123

Description

The update function in the contract is callable by any external address. The function calls _updateUserState, which modifies the internal staking state of the caller (_msgSender) for the specified _asset.

This design introduces several potential risks:

  1. Unintended State Changes:
    • Malicious or careless users can call update unnecessarily, causing additional state changes and emitting events. While this does not directly lead to fund loss, it can increase the gas cost and make the state history more convoluted.
  2. Excessive Gas Consumption:
    • Public access to update may result in unnecessary calls, wasting computational resources and increasing gas costs for the protocol.
  3. DoS Risks:
    • If the update function is abused at scale, it could create excessive on-chain activity, potentially hindering other users' operations on the contract.

Exploit Scenario

  1. Scenario 1:
    • A malicious user or bot repeatedly calls update for multiple assets, incurring high gas costs and unnecessarily bloating the transaction history with emitted events.
  2. Scenario 2:
    • A user intentionally or accidentally calls update with invalid or disabled assets. While this would revert due to validation, repeated reverts could disrupt legitimate activity and waste computational resources.

POC

  1. Copy and paste the following foundry test function in this file: contracts/test/voting/VotingPool.t.sol
  2. Then save and run forge test in the following folder: contracts/
function testUpdateByAnyone() public {
        address anyOne = address(0xdeefFeef);
        address asset1 = address(pairedLpToken);
        vm.startPrank(anyOne);
        votingPool.update(asset1);
        vm.stopPrank();
    }
  1. The test then passes for any tom dick and harry. The log results of the foundry test follows.
forge test --match-test testUpdateByAnyone
Ran 1 test for test/voting/VotingPool.t.sol:VotingPoolTest
[PASS] testUpdateByAnyone() (gas: 65208)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.90ms (184.04µs CPU time)

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

Recommendations

  1. Access Control:
    • Restrict the update function to only allow whitelisted or approved entities to call it. For example:
+         function update(address _asset) external onlyOwner returns (uint256 _convFctr, uint256 _convDenom) {
-         function update(address _asset) external returns (uint256 _convFctr, uint256 _convDenom) {
        return _updateUserState(_msgSender(), _asset, 0);
    }