It is my understanding after discussing with the sponsor that calls from the terminal are atomic which makes impact here negligable, however, the terminal contract and it's implementation is out of the scope of this audit.
Key variables (mintedAmount and reservedRate) used in didPay() can be tainted by a malicious user's call to payParams().
Proof of Concept
Terminal calls payParams() to set pay parameters
Malicious user calls payParams() to taint mintedAmount and reservedRate state variables
Terminal calls didPay() which uses tainted mintedAmount and reservedRate state variables
Recommended Mitigation Steps
Add access control to didPay() similar to payParams():
if (msg.sender != address(jbxTerminal)) revert JuiceBuyback_Unauthorized();
Lines of code
https://github.com/code-423n4/2023-05-juicebox/blob/9d0458282511ff269b3b35b5b082b56d5cc08663/juice-buyback/contracts/JBXBuybackDelegate.sol#L144
Vulnerability details
Malicious actors can frontrun calls to
didPay()
with a call topayParams()
to taint state variablesmintedAmount
andreservedRate
Anyone can observe the mempool and frontrun other's calls to didPay() with a call to JBXBuybackDelegate.payParams().
This malicious call to
payParams()
can set variablesmintedAmount
andreservedRate
@ JBXBuybackDelegate.sol#L158-L159These variables are used in didPay() and passed to _swap()/_mint() @ L205 and _mint() @L207
Impact
Key variables (
mintedAmount
andreservedRate
) used indidPay()
can be tainted by a malicious user's call topayParams()
.Proof of Concept
payParams()
to set pay parameterspayParams()
to taintmintedAmount
andreservedRate
state variablesdidPay()
which uses taintedmintedAmount
andreservedRate
state variablesRecommended Mitigation Steps
Add access control to
didPay()
similar topayParams()
:if (msg.sender != address(jbxTerminal)) revert JuiceBuyback_Unauthorized();
Assessed type
Timing