Flow

Description

location : /src/abstracts/Batch.sol

The batch function allows any external user to supply arbitrary function calls that are executed using delegatecall within the context of the contract itself.

code :

Issue:

• Unrestricted delegatecall: The delegatecall function is being used on address(this) with calldata provided directly by the caller without any restrictions or validation.

• Bypassing Access Control: Attackers can craft calldata to call any function in the contract, including internal or private functions, and even functions restricted by access modifiers like onlyOwner or onlyAdmin, if those access checks rely on msg.sender.

• Execution of Unauthorized Actions: Since delegatecall maintains the original msg.sender and msg.value, if access control modifiers rely solely on msg.sender, attackers may execute privileged functions.

• Reentrancy Attacks: The use of delegatecall may expose the contract to reentrancy vulnerabilities, especially if combined with functions that modify state or transfer funds.

• State Manipulation: Attackers can manipulate the contract's state by calling setter functions or modifying storage variables, potentially leading to loss of funds or denial of service.

Exploit Scenario

1. Bypassing onlyAdmin Modifier:

   Suppose the contract that inherits Batch has a function protected by onlyAdmin:

   function setParameter(uint256 _value) external onlyAdmin {

   parameter = _value;

   }

   An attacker can craft calldata for setParameter and include it in the calls array passed to batch. Since delegatecall preserves msg.sender, and if onlyAdmin relies on msg.sender, the attacker may not pass the check unless they are not the admin.

   However, if there is any flaw in the access control logic or if there are internal functions that lack access modifiers, the attacker can exploit this.

2. Calling Internal Functions:

   Internal functions are not supposed to be callable externally. However, using delegatecall, an attacker can invoke these functions by supplying the appropriate function signature in the calldata.

3. Reentrancy via State Changes:

   If the contract has payable functions or functions that transfer tokens or Ether, an attacker can use batch to perform reentrant calls, potentially draining funds.

Impact

• Unauthorized Access: Attackers can perform actions that should be restricted, leading to privilege escalation.

• Loss of Funds: Exploiting state-changing functions can lead to theft of tokens or Ether from the contract.

• Contract Hijacking: Attackers can manipulate critical state variables, changing the behavior of the contract to their advantage.

• Denial of Service: Malicious calls can put the contract into an unusable state, affecting all users.

Recommendation

Restrict or Remove the Use of delegatecall with Untrusted Input

1. Avoid Using delegatecall with External Input:

   Do not use delegatecall with calldata supplied by external users. If batching is necessary, use call instead, which does not delegate context and is safer.

2. Validate Function Calls:

   If delegatecall must be used, implement strict validation to ensure only allowed functions can be called, and that the calldata does not include unauthorized function signatures.

3. Implement Access Control:

   Ensure that all sensitive functions have robust access control checks that cannot be bypassed by manipulating msg.sender or through reentrancy.

4. Use reentrancyGuard:

   Protect state-changing functions with a reentrancy guard to prevent reentrant calls.

5. Alternative Batching Mechanism:

   Consider using a different approach for batching, such as:

   • Whitelist of Functions: Maintain a whitelist of functions that can be batched, and reject any calls to functions not on the list.

   • Use of Function Selectors: Parse the calldata to extract function selectors and ensure they match allowed functions.

   • Separate Contract for Batching: Implement batching in a separate contract with tightly controlled functionality.

Revised Code Example

Here is a safer version of the batch function using call:

• Note: Using call instead of delegatecall means that the code is executed in the context of address(this), but without modifying the storage of the calling contract, and msg.sender would be the contract itself, not the original sender.

• Implication: If functions rely on msg.sender for access control, switching to call may affect their behavior. Adjustments to access control logic may be necessary.