Reentrancy issues on function `distributePayoutsOf`

[code423n4]

Lines of code

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L415-L448 https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L788-L900 https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L981-L1174 https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBETHPaymentTerminal.sol#L63-L79 https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/JBERC20PaymentTerminal.sol#L73-L89

Vulnerability details

Impact

In the contract JBPayoutRedemptionPaymentTerminal, the function distributePayoutsOf calls the internal function _distributePayoutsOf and this internal function perfoms a loop where is using the function _distributeToPayoutSplitsOf In these functions there are a _transferFrom what:

• JBETHPaymentTerminal using a Address.sendValue(_to, _amount)
• JBERC20PaymentTerminal using a IERC20(token).transfer(_to, _amount) with a ERC777 as token

Both give back the control to the msg.sender (_to variable) creating a reentrancy attack vector

Also could end with a lot of bad calculation because is using uncheckeds statements and function _distributePayoutsOf its no respecting the checks, effects, interactions pattern

Proof of Concept

Craft a contract to call function distributePayoutsOf, on receive ether reentrant to function distributePayoutsOf or use a ERC777 callback.

Tools Used

Manual Review

Recommended Mitigation Steps

Add a reentrancyGuard as you do on JBSingleTokenPaymentTerminalStore.sol; You have already import the ReentrancyGuard on JBPayoutRedemptionPaymentTerminal.sol#L5 but you are not using it.

My recommendation is to add nonReentrant modifier on function distributePayoutsOf

[simon-something]

Duplicate of #35

[jack-the-pug]

Lack of clear path to exploit it, but it dose seems like _distributeToPayoutSplitsOf can be used to reenter distributePayoutsOf; it requires the attacker to be one of the project's splits beneficiaries, though.

https://github.com/jbx-protocol/juice-contracts-v2-code4rena/blob/828bf2f3e719873daa08081cfa0d0a6deaa5ace5/contracts/abstract/JBPayoutRedemptionPaymentTerminal.sol#L1148-L1153

          _transferFrom(
            address(this),
            _split.beneficiary != address(0) ? _split.beneficiary : payable(msg.sender),
            _netPayoutAmount
          );