A subtle reentrancy in a flashloan path
A finding that bypassed the standard reentrancy guard by exploiting an external call inside a flashloan callback. Walkthrough of the path, the fix, and why the audit missed it.
Most reentrancy bugs are caught by the standard nonReentrant modifier. This one was not. The contract had the modifier on every state-changing function. The bug was inside a callback the contract made to a flashloan provider, which then called back into the contract through a path that did not have the modifier.
The setup
The protocol accepted user deposits and used them as liquidity for a flashloan provider. When a flashloan was taken, the protocol received the principal plus a fee. The fee was sent to the protocol's treasury through a hook.
The hook was a public function. It was not marked nonReentrant because it was supposed to be called only by the flashloan provider.
The path
function onFlashLoan(...) external {
require(msg.sender == FLASHLOAN_PROVIDER);
treasury.deposit(fee);
}
The treasury contract had its own deposit function. That function, in some edge cases, called an external hook on the caller. The caller was the protocol. The hook on the protocol was the same onFlashLoan. A malicious flashloan provider could trigger this loop.
The impact
The fee could be credited multiple times before the first call returned. With a large enough flashloan, an attacker could drain the treasury in a single transaction.
The fix
Mark onFlashLoan as nonReentrant. Or better, pull the fee credit out of the callback entirely and settle it after the flashloan returns. Defense in depth, not defense in breadth.
Why the audit missed it
The audit was scoped to the protocol contract. The treasury was listed as a trusted external dependency. The hook on the treasury was not in scope. This is a classic case where the bug lives on the seam between two contracts that are each safe on their own.