Repository navigation
FLO-29: Per-Position Reentrancy Lock Does Not Protect Shared Pool State During External Callbacks #238
Description
Severity: Low
Files Affected
cadence/contracts/FlowALPv1.cdc
Description
The reentrancy guard (_lockPosition/ _unlockPosition, lines 1572–1583) locks by position ID, not at the pool level. While position A is locked and making external calls (to oracle, DEX swapper, sink, or source), any other position B can freely operate on shared mutable pool state — reserves, TokenState balances, interest indices/rates, and deposit capacity.
Recommendation
If the intent is to protect shared state during external callbacks, consider either (a) a pool-level lock that prevents all position operations during any external call, or (b) locking the affected TokenState and reserve vaults in addition to the position. A lighter alternative is to snapshot shared state before the external call and validate it hasn't changed after the call returns, reverting if it has.
Parent Issue: #209
Activity
holyfuchs commentedon Mar 13, 2026
While the reentrancy guard is scoped per position ID, we did not identify a concrete attack vector that would allow this to be exploited to manipulate shared pool state. As such, this is a theoretical concern rather than a practical vulnerability.
That said, we have not exhaustively analyzed all possible multi-position interaction patterns and may review this further. Introducing a pool-level lock would also restrict legitimate interactions involving multiple positions within the same transaction, so there are some design tradeoffs to consider.
jordanschalm commentedon Apr 9, 2026
I'm in favour of adding the pool-level lock.
Most legitimate transactions which operate on multiple positions in one transaction would still function with a pool-scoped lock. Any sequence of operations -- where each operation finishes before the next starts -- on multiple positions will work fine. The only thing that won't work is when a position operation uses a dependency (Sink, Source, Swapper, etc) that then calls back into the Pool. I don't think there is a clear need for supporting this kind of pattern.
For this kind of high-assurance software project, we need to provide affirmative evidence that the software behaves as expected under all conditions. If we don't know that it's safe, we need to assume it's not. I think the audit suggestion is appropriate.