code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-juicebox/blob/main/juice-buyback/contracts/JBXBuybackDelegate.sol#L232 https://github.com/code-423n4/2023-05-juicebox/blob/main/juice-buyback/contracts/JBXBuybackDelegate.sol#L286

Vulnerability details

Impact

The contract in scope has a withdraw function namely ‘uniswapV3SwapCallback’ which sends funds to the calling address. The calling address can be a malicious contract. Currently transfer() sends more gas than 2300 creating a potential attack vector for re-entrancy.

When the malicious contract call the withdraw function, before reducing the balance of the malicious contract, it's fallback function will be called at it can steal more funds from the contract in scope.

Proof of Concept

Check this artilce by Consensys https://consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/.

Recommended Mitigation Steps

Recommendation is made for using .call() method which is more gas efficient, as it forwards all remaining gas, as in combination with re-entrancy guard to use after December 2019. Exception propagation for .call() method can be achieved with a workaround using the re-entrancy Guard Check pattern. The pattern for Re-entrancy Guard should be by making all state changes before calling other contracts or either using re-entrancy guard modifier.