Unsafe cast in swap and uniswapV3SwapCallback functions can lead to attack

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-juicebox/blob/9d0458282511ff269b3b35b5b082b56d5cc08663/juice-buyback/contracts/JBXBuybackDelegate.sol#L224 https://github.com/code-423n4/2023-05-juicebox/blob/9d0458282511ff269b3b35b5b082b56d5cc08663/juice-buyback/contracts/JBXBuybackDelegate.sol#L271

Vulnerability details

Impact

The swap and uniswapV3SwapCallback functions performs an unsafe cast of a uint256 type to a signed integer.

_amountReceived = uint256(-(_projectTokenIsZero ? amount0 : amount1));

Note that amount is chosen by the caller and when choosing amount = 2**256 - 1, this is interpreted as 0xFFFFFFFFF... = -1 as a signed integer. Thus -(-1)=1 adds 1 liquidity unit to the position

This allows an attacker to not only swap and send the non-reserved token to the beneficiary, amounts according to the unmodified uint256 amount which is an extremely large value and calculated for _nonReservedToken calculation and is transferred to the beneficiary.

Proof of Concept

https://solodit.xyz/issues/843

Tools Used

Manual Review

Recommended Mitigation Steps

Even though Solidity 0.8.x is used, type casts do not throw an error. A SafeCast library must be used everywhere a typecast is done.

Assessed type

Under/Overflow

[c4-pre-sort]

dmvt marked the issue as low quality report

[dmvt]

overinflated... no actual example attack shown

[c4-judge]

dmvt marked the issue as unsatisfactory: Overinflated severity