Harvest Finance Uninitialized Proxies Bugfix Review— $200k Bounty

Published in

Immunefi

12 min readNov 12, 2021

Software development is an iterative process, and mistakes can happen at any time. That’s why, in the practice of developing software, there is usually a team of QA Engineers — who act as a second pair of eyes for developers. In Web2, these bugs are usually quickly patched and the problem would go away. However, in Web3, it’s not that simple.

What happens when you have an immutable blockchain and have a bug to patch? Unlike regular code, smart contracts do not have a built-in method of being patched or updated in place.

This creates some amount of security for smart contracts since they can’t suddenly change their properties. A contract that lets you lend your tokens, for example, won’t suddenly empty your account, because the code of that contract cannot be changed wholesale.

But it also creates a problem for developers and users when a vulnerability is found on a deployed smart contract — with potential millions of dollars at risk, how do you fix a contract without being able to alter its code?

One of the solutions devised for this is called the Upgradable Proxy Pattern, an ingenious method that solves the initial concern of non-upgradeable contracts, but also introduces a new set of new issues — so developers have to be extra careful. If it is performed wrongly, you could end up with a dead contract and stranded user balances.

Luckily, auditors and bug bounties in Web3 have encouraged amazing people to step forward and report these vulnerabilities responsibly, like the bigbrains at Dedaub, who reported this interesting bug.

Summary

The Dedaub team, auditors and creators of the tool https://contract-library.com/, filed a submission via Immunefi for uninitialized implementation contracts for Uniswap V3 vault proxies found in the well-known Ethereum protocol, Harvest Finance.

This critical bug could have led to the self-destruction of the implementation contract, which could have rendered the proxy contracts useless. This is because of the upgradeable proxy pattern used: one with the upgrade logic residing within the implementation contract rather than the proxy.

This is very similar to the recent OpenZeppelin UUPS vulnerability, but the Harvest contracts were not using the OpenZeppelin code. In fact, the automated analysis that detected the vulnerable contracts asked exactly this question: “how can we generalize the logical elements of the OpenZeppelin UUPS uninitialized-proxies bug, to find similar issues in different code?” As we will see, the code is quite different, but has elements that enable the same attack.

The contracts in scope held a total of $6.4M in Uniswap V3 positions at the time of the submission. The Harvest Finance team acknowledged the bug and quickly patched the issue.

Dedaub was paid $100,000 by Harvest Finance, and an additional $100,000 from Armor, due to Harvest’s participation in Armor Finance’s bug bounty matching program.

Before we dive into the nitty-gritty of the bug, let’s have a quick review of contract proxies.

Intro to Proxies

It is logical that all code, even immutable smart contracts, may eventually need to be upgraded. This is especially true as a safeguard against newly-discovered vulnerabilities and for adding new features to the protocol. But there is some disagreement from developers on which specific pattern of upgrade mechanism is best.

Introducing the ability to upgrade contracts adds a lot of complexity to the process, and for some, defeats the purpose of a blockchain’s immutability, or the decentralization of control over smart contracts.

A smart contract upgrade can be simply summarized as: a change in the code at a specific address while preserving the storage state of a previous code.

Preserving storage state is necessary as we want to have access to all of the state changes that happened before (i.e. history of interactions), but we want to change the code that is governing the logic of its interactions. Another way of saying this is that we are only swapping the implementation, not the state of the contract.

We can achieve this by using a proxy contract and delegate calls.

Proxy and DELEGATECALL

In Ethereum, there are three major types of a contract calls. A regular CALL, STATICCALL and DELEGATECALL.

When contract A makes a CALL to contract B by calling foo(),the function execution relies on contract B’s storage, and the msg.sender is set to contract A.

This is because contract A called the function foo(), so that the msg.sender would be contract A’s address and msg.value would be the ETH sent along with that function call. Changes made to state during that function call can only affect contract B

However, when the same call is made using DELEGATECALL, the function foo() would be called on contract B but in the context of contract A. This means that the logic of contract B would be used, but any state changes made by the function foo() would affect the storage of contract A. In this case, msg.sender would point to the EOA who made the call in the first place. (See example 2)

A delegatecall makes it possible to create proxy contracts using a proxy pattern. This way, the proxy contract redirects all the calls it receives to an implementation contract, whose address is stored in its (Contract A’s) storage. From a user perspective, the proxy contract runs the implementation contract’s code as its own, modifying the storage and balance of Contract A, the proxy contract. (See example 3)

Making an upgrade in this case is quite simple, as we only need to change the stored implementation contract address for the proxy in order to change its smart contract logic. All incoming calls will be redirected to the new address, and nothing changes from the user’s perspective.

Another thing we need to take into account is: how can we handle the constructor logic? The contract’s constructor is automatically called during contract deployment. Most developers would put the initialization logic there, in order to make the smart contract functions correctly.

But this is no longer possible when proxies are in play, as the constructor would change only the implementation contract’s storage, not the storage of the proxy contract, which is the one that matters.

Therefore, an additional step is required. We need to change the constructors to a regular function. This function is conventionally called initialize or init. These are regular Solidity functions that are added to the implementation contract and, when called from the proxy, change the proxy contract’s storage. They also need special logic to ensure they can only be called once, similar to a constructor.

There are two major ways to implement this Proxy and delegate call pattern. We illustrate using the modern OpenZeppelin specifics and terminology, although the details in the case of the Harvest finance code were different.

Transparent Proxy Pattern (TPP) and Universal Upgradeable Proxy Standard (UUPS)

For the proxy method described above, there are some major issues. For example, when a proxy admin wants to call a proxy contract function upgradeTo which shares a name with a function in the implementation contract, which one would be called? This sort of conflict can lead to unintended behaviors or even malicious exploitation.

There are a few solutions to avoid this issue. The first one is called the Transparent Proxy Pattern (TPP). This method makes it so that all calls by a user always execute using the implementation contract’s logic instead of the proxy contract’s logic. Calls by the admin always execute using the proxy contract’s logic.

In a scenario where a user would call a function upgradeTo which shares a name in both contracts, she can be sure that the logic from the implementation will be executed, and not the proxy’s.

But what about the proxy’s admin? We would still want to be able to call proxy’s upgradeTo function when needed. The solution to the whole issue is to assign one address as the owner, to deploy and manage the proxy. This also ensures that, when a call isn’t made from the admin, the implementation contract is called instead. The following diagram shows an example of scenarios that could happen.

(source:https://i2.wp.com/blog.openzeppelin.com/wp-content/uploads/2018/11/Proxy_scenarios.png?resize=659%2C93&ssl=1)

However, this solution is not without its drawbacks. The transparent proxy still needs additional logic implemented within the proxy contract to manage all the upgradability functions, as well as the ability to identify whether the caller is the admin address. This involves reading the storage state, as well as executing additional logic which increases the execution cost of the contract, and is not as gas efficient.

Although TPP is still widely used, attention is starting to shift towards an alternative called UUPS.

The main difference between the two is which contract contains the upgrade logic. As we know, with TPP, upgrade logic is located in the proxy contract itself. But with UUPS, the logic is situated in the implementation contract. Calling upgradeToAndCall() from the proxy to the implementation contract will cause the change in the implementation address to be reflected in the proxy itself. This works because UUPS implementations have access to all the storage of the proxy; they can overwrite the storage slot of the proxy contract where the proxy stores the address of the implementation.

This simple change alone makes proxy calls cheaper, because we only check that the caller is the admin when an upgrade is requested. We also don’t need to have logic for the case where there are two functions with the same name. The code generated automatically by Solidity in the implementation contract takes care of this for us. All authorization logic for upgradability is located within the implementation contract to guard against any unintended calls from happening.

Another distinction is how upgrade logic behaves. In order to ensure the new upgraded contract is also able to be upgraded in future, the upgradeTo() function also performs a “rollback” check to ensure that we don’t accidentally upgrade to a contract that can’t be upgraded further.

For a more in-depth analysis of differences between TPP and UUPS, it is recommended to read OpenZeppelin’s explanation on the topic: https://docs.openzeppelin.com/contracts/4.x/api/proxy#transparent-vs-uups.

OpenZeppelin UUPS Uninitialized Proxies Vulnerability

Before we look at the Harvest Finance vulnerability, we discuss the recent OpenZeppelin UUPS vulnerability, which is very closely related but affected a lot more deployed contracts. Although the code is different, the Harvest vulnerability was detected by generalizing the pattern of the OpenZeppelin UUPS vulnerability.

As mentioned previously, when UUPS proxy contracts are deployed, the constructor isn’t implemented in the implementation contract. The implementation provides the initialize() function as a substitute. In many cases, developers also use upgradeable versions of the standard OpenZeppelin contracts which implement their own initialize() functions.

The below example is taken from OpenZeppelin’s security advisory post.

// SPDX-License-Identifier: MITpragma solidity ^0.8.2;import “@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol”;
import “@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol”;
import “@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol”;
import “@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol”;contract MyToken is Initializable, ERC20Upgradeable, OwnableUpgradeable, UUPSUpgradeable {
  function initialize() initializer public {
    __ERC20_init(“MyToken”, “MTK”);
    __Ownable_init();
    __UUPSUpgradeable_init();
  }function _authorizeUpgrade(address newImplementation)
    internal
    onlyOwner
    override
  {}}

We can see the initialize() function calls __Ownable_init, which sets the owner of the implementation contract to the first person to call it. This is a key point.

Being an owner of the UUPS implementation contract means you can control the upgrade functions. In particular, the owner of the implementation can call upgradeToAndCall() directly on the implementation contract, instead of going through the proxy.

The vulnerability lies in how upgradeToAndCall() works internally. Apart from changing the implementation address to a new one, it atomically executes any migration/initialization function using DELEGATECALL and the data passed along it. If the initialization function of the new implementation executes the SELFDESTRUCT opcode, the DELEGATECALL caller (the implementation contract) will be destroyed. This happens because updateToAndCall() is using DELEGATECALL, and in the case of calling this function directly, SELFDESTRUCT is executed in the context of the implementation contract.

This would cause the proxy contract to become useless, as it would forward all the calls to an empty address. Upgrading would no longer be possible, nor can you switch the upgrade mechanism to fix this, as the upgrade logic is hosted on the implementation contract by design of the UUPS pattern.

Here is a step-by-step guide of how a hypothetical attack could be performed:

The attacker calls initialize() on the implementation contract to become the owner.
Attacker deploys a malicious contract with a selfdestruct() function.
The attacker calls upgradeToAndCall() on the implementation contract as an owner, and points it to the malicious implementation contract.
During the upgradeToAndCall() execution,DELEGATECALL is called from the regular implementation contract to the malicious implementation contract
SELFDESTRUCT is called destroying the regular implementation contract.
The proxy contract is now rendered useless.

Harvest Vulnerability Analysis

On October 20th, the Dedaub team submitted a report to Immunefi detailing 3 uninitialized proxy implementations from Harvest Finance.

Interestingly, these implementation contracts have no published source code. None of the three contracts are verified on Etherscan, and only the bytecode of each contract is publicly available. Given Dedaub’s expertise in decompilation and analysis of contracts, this was but a small obstacle.

The above implementations were used in proxies that had significant holdings. A total of $6.4M was at risk at the time of the submission. For instance, this proxy held $2M in a Uniswap v3 position:
https://contract-library.com/contracts/Ethereum/1851A8FA2CA4D8FB8B5C56EAC1813FD890998EFC

Unlike the implementation, the proxy does have publicly available source code. It is again an upgradeable proxy. Unlike the OpenZeppelin UUPS pattern, the upgrade logic does not reside in the implementation contract. However, the implementation contract is consulted during the upgrade:

function upgrade() external {
  (bool should, address newImplementation) = IUniVaultV1(address(this)).shouldUpgrade();
  require(should, “Upgrade not scheduled”);
  _upgradeTo(newImplementation);
 …

The call shouldUpgrade() is handled by the implementation contract. Therefore, we have again the same issue as in the UUPS pattern: should the implementation self-destruct, the proxy contract is incapacitated and cannot update its implementation!

But how can the implementation self-destruct? The Harvest code does not have the upgradeToAndCall functionality of the OpenZeppelin code. However, if one consults the decompiled code of the implementation contract (since there is no published source) an analogous threat is apparent: the Harvest logic delegatecalls a “hard worker” contract, to perform various tasks, such as the doHardWork() yield farming functionality. Therefore, all an attacker has to do is:

initialize the implementation contract
set the hard-worker storage field to a contract that calls SELFDESTRUCT
call doHardWork()

This destroys the implementation and incapacitates the proxy.

The attack was found by an automated analysis that attempted to generalize the elements of the OpenZeppelin UUPS uninitialized implementation vulnerability. The general elements that the analysis identifies are:

A contract X that, if uninitialized, allows any untrusted caller to DELEGATECALL an address of the caller’s choosing. This is a deep, complex static analysis of the contract’s code.
The contract X has an initializer (i.e., function init, initialize, with any signature variation, for a total of over-250 signatures examined). This is a straightforward static analysis.
The initializer has not been called in the past. This is a dynamic analysis over past transactions.
A different contract Y has DELEGATECALLed X in the past. Extra conditions can include that Y calls X during its upgrade calls, for more precision, but this condition was not even used when the Harvest contracts were flagged.

Vulnerability Fix

A fix was implemented by repointing the proxies to implementation contracts that cannot selfdestruct at the command of untrusted callers.

Acknowledgments

We want to thank the Dedaub for the very interesting submission, and congratulate them on having the skill and dedication to find such an issue within unverified contracts.

If you’d like to start bug hunting, we got you. Check out the Web3 Security Library, and start earning rewards on Immunefi — the leading bug bounty platform for web3 with the world’s biggest payouts.

We also want to thank the Harvest Finance team for their swift response, and expert handling of the issue. To report additional vulnerabilities, please see Harvest Finance’s bug bounty program with Immunefi. If you’re interested in protecting your project with a bug bounty, visit the Immunefi services page and fill out the form.