Flow

Summary

The NoDelegateCall contract's logic for preventing delegate calls may allow unauthorized access if not implemented correctly, compromising the contract's security.

Finding Description

The _preventDelegateCall() function checks whether the current contract instance (address(this)) matches the ORIGINAL address, which is set to the contract's own address during construction. If an attacker can manipulate the execution context, such as by using delegatecall from a contract that inherits from NoDelegateCall, they could bypass this check. This situation arises because the delegate call context will still have address(this) equal to the address of the calling contract, rather than the expected original contract.

This breaks the security guarantee of ensuring that only the original contract can execute functions protected by the noDelegateCall modifier. If a malicious actor can trigger a delegate call to the contract, they could execute restricted functions, leading to unauthorized access and potential exploitation.

Vulnerability Details

The vulnerability arises from the assumption that address(this) will always represent the original contract that was deployed. When a derived contract calls a function that uses the noDelegateCall modifier via a delegatecall, address(this) will point to the derived contract, not the original NoDelegateCall contract, thus bypassing the intended security measure.

Impact

This vulnerability can lead to significant security risks, as it allows derived contracts or external actors to execute functions that should be restricted to the original contract only. If sensitive functions or state changes are accessible via delegate calls, it could result in loss of funds, unauthorized changes to the contract state, or other harmful effects.

Proof of Concept

A proof of concept could be demonstrated with the following example:

1. Attacker Contract:

   contract Attacker {

   NoDelegateCall public target;

   ​

   constructor(NoDelegateCall _target) {

   target = _target;

   }

   ​

   function exploit() external {

   // Attempt to call a restricted function on NoDelegateCall

   target.call(abi.encodeWithSignature("restrictedFunction()"));

   }

   }

2. Execution:
   If restrictedFunction() is protected by the noDelegateCall modifier, the attacker's call will pass the check because address(this) will equal the attacker's contract address, not the original.

Recommendations

To fix this issue, the contract should be modified to ensure that the delegate call prevention logic accurately checks the caller's context. One approach is to check the msg.sender instead of relying solely on the ORIGINAL address.

Suggested Code Fix

Additionally, consider whether using ORIGINAL is necessary or if the check should simply confirm that the function is not called via delegate call by validating the msg.sender against known authorized addresses.

File Location

NoDelegateCall.sol