fix
Menu

Reentrancy Guard 2.0

How we could have stopped tens of millions of $ from being stolen, and accidentally made 2.5K$

5 min read
Table of contents

The Persistent Problem of Reentrancy Attacks

Starting our journey few months ago, we began with a comprehensive survey of smart contract security challenges and solutions. The reentrancy attack, arguably the most common and famous type of attack, was there from the get go. The ReentrancyGuard by OpenZeppelin has been around at least since beginning of 2017 to mitigate these kind of attacks. So we asked ourselves, how come a bug with such an easy mitigation still causes so much havoc?

Interestingly enough, we’ve noticed something even more disturbing: protocols that had already been implementing ReentrancyGuard, to some extent, are still being hacked by reentrancy!

Cream ($18.8m), Fei-Rari ($80m), BurgerSwap ($7.2m), and Orion ($3m), to name a few. All had a reentrancy guard in place, but failed to prevent the malicious reentrant call.

Understanding the Limitations of Traditional Reentrancy Guards

We scratched our heads in bewilderment and tried to triage this.

At first we thought it might be some kind of bug in the ReentrancyGuard contract, but it was not. It is a fairly simple concept and OZ made a tremendous job in refining it.

We came to the conclusion that the traditional reentrancy guard has some fundamental limitations that prevents it from providing a perfect prevention against reentrant calls:

  • public functions. ReentrancyGuard includes a global flag, shared between all functions.If developers intend to call a public function internally they’ll probably choose not to protect it (Since protection will result it being blocked both externally and internally)
  • Inheriting from a contract already implementing ReentrancyGuard , making the main contract reentrancy inheritance somewhat cumbersome
  • view functions. The reentrancy check includes saving to storage, making it unusable in the context of a STATICCALL
  • Overall integration complexity. Although the ReentrancyGuard is a very straightforward contract, it requires the developer to add a modifier to each protected function. What happens if there are multiple developers? what happens in case of protocol upgrades?

As cyber security experts, we know that security counter measures are only as good as how it is being implemented, and this is exactly our way of thinking at SphereX.

A New Approach: Incorporating Reentrancy Guard in the Proxy Contract

We asked ourselves, is there a better implementation pattern for this crucial security element? Is there somehow a way for it to be incorporated in such a way that will prevent all the above problems and still be easily used?

The answer is yes!

Most of the protocols mentioned above were also using a proxy. Let’s imagine a conceptual protocol where a proxy is being used and the implementation is protected by a reentrancy guard:

In this traditional design every function in the implementation is accompanied with a nonReentrant modifier. Every implementation upgrade might break the consistency of the reentrancy check and might introduce new reentrancy bugs.

We would like to suggest a different approach. For the reentrancy guard to be incorporated in the proxy contract rather than in the implementation:

This way, we have one, non-distributed enforcement point to prevent Reentrancy!

This approach has some other cool features:

  • It requires no extra coding! the proxy contract comes as is, doesn’t require any pre-deployment integrations, and now that reentrancy guard is in the proxy the implementation contract becomes leaner and cleaner.
  • It’s upgrade-proof! any upgrades to the protocol are on the implementation part, which will always be protected of reentrant calls by the proxy
  • public functions are protected! developers can now call public functions internally with ease of mind. External reentrant calls to public functions are protected using this design
  • view functions are protected! even in the context of a STATICCALL as special check in the proxy will prevent “read-only reentrancy”

We took the liberty of implementing this. We took ReentrancyGuard and TransparentUpgradeableProxy as reference contracts and used them to do exactly that. This wasn’t without challenges (That you can read about at the bottom).

Interesting story, we decided to fully implement this during the awesome ETHDenver 2023 Hackathon. Thinking that the odds were small, we presented the project in the morning of the last day of the Hackathon, and then headed to the airport to catch a flight.

We watched the live broadcast of the closing ceremony finding out we were nominated as one of the best infrastructure projects in the Hackathon 😞

We felt like this is such a fundamental piece we have to release it for everyone to benefit, but keep in mind, this is the bare minimum. Web3 hacks are becoming more and more sophisticated and our analysis shows the current tools at hand just don’t cut it.

Some technical difficulties we were facing

  • The reentrancy flag in the reentrancy guard is stored in the contract storage. Proxies need to be careful using the storage since it is shared with the implementation. For this reason we moved the reentrancy flag to a very high storage slot
  • The proxy code is the same whether the current context is CALL or STATICCALL, since the reentrancy guard mechanism alters the storage, which is prohibited in STATICCALL, we have to check first this is the case and act accordingly.
  • OZ proxies work in such a way that after the implementation call it returns immediately (roughly speaking), we had to change this behavior to be able to reset the reentrancy flag on exit

Conclusion

Some caveats you should know before using this:

  • We currently still haven’t implemented any fine-grained control of the function selectors that can pass the reentrancy guard. This means that callbacks, where the contract calls a 3rd party which in turn calls a callback function in the contract, will trigger the reentrancy guard.
  • One legitimate use-case using this scenario is a contract transferring NFT to itself, which requires triggering onERC721Received.
  • As with normal proxies, send and transfer of funds will probably revert due to the 2300 gas limit it creates

We’re releasing the reentrancy proxy for free use by the community, for a better secured Web3.0.

Here it is:

https://github.com/spherex-xyz/reentrancy-guard-proxy

About the author

Ariel Tempelhof
CPO @SphereX's
Follow

Ariel has over 12 years of software development and cybersecurity research experience. Before joining SphereX, Ariel was the co-founder and CEO of Realmode Labs, a boutique cybersecurity research firm.

Continue your reading with these value-packed posts
Go back to blog