Unchecked return value of low level

[c4-submissions]

Lines of code

https://github.com/delegatexyz/delegate-registry/blob/38576a79bd3781cc719bf7e6ff242cba4420ca8c/src/DelegationRegistry.sol#L190

Vulnerability details

In the code you provided earlier, there is a potential "Unchecked return value of low-level call()" vulnerability in the following line:

address(strategy).delegatecall(abi.encodeWithSignature("harvest()"));

This line of code uses the delegatecall function to invoke the "harvest()" function in an external contract referred to as strategy. The vulnerability arises because the return value of this delegate call is not checked for success or failure.

Here's an explanation of the issue:

1. Delegatecall Function: In Solidity, the delegatecall function is used to execute code from an external contract within the context of the calling contract. It's a powerful feature, but it also carries certain risks.

2. Unchecked Return Value: The problem in this code is that the return value of the delegatecall is not captured, and there is no check to determine whether the call was successful or not.

3. Potential Consequences:

   • If the delegatecall fails, it can have unintended consequences, such as altering the state of the calling contract or causing it to behave incorrectly.
   • It might also lead to a loss of control over the contract's state and funds if the external strategy contract behaves maliciously or unexpectedly.

To mitigate this vulnerability, it's essential to check the return value of the delegatecall and handle any potential errors. Here's an example of how you can improve this code:

(bool success, ) = address(strategy).delegatecall(abi.encodeWithSignature("harvest()"));
require(success, "Delegatecall to strategy's harvest() failed");

In this improved code, we capture the return value of the delegatecall in the success variable and use a require statement to check if the call was successful. If the call fails, it will revert the transaction, preventing any unintended consequences.

In summary, the "Unchecked return value of low-level call()" issue in the code you provided is a potential vulnerability because it doesn't handle the possibility of the delegatecall failing. Checking and handling the return value is crucial to ensure the safety and integrity of the smart contract.

Assessed type

Other

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid

[GalloDaSballo]

Wrong file