Trader Joe v2 contest - ElKu's results

Lines of code

https://github.com/code-423n4/2022-10-traderjoe/blob/79f25d48b907f9d0379dd803fc2abc9c5f57db93/src/LBRouter.sol#L520

Vulnerability details

Impact

If the user doesnt send the exact amount of ether while calling the swapAVAXForExactTokens function, it will revert. Since its impossible to know what is the exact amount of ether needed for the function to work before hand, the user will never be able to swap AVAX.

Proof of Concept

Looking at the code of the swapAVAXForExactTokens function, we can see that if the msg.value < amountsIn[0], then the function will revert.

Otherwise if all other conditions pass, it will arrive at the last statement in the function:

if (msg.value > amountsIn[0]) _safeTransferAVAX(_to, amountsIn[0] - msg.value);

If the msg.value > amountsIn[0], then the condition passes and we call the _safeTransferAVAX function with two arguments. The second argument is amountsIn[0] - msg.value, which will ALWAYS revert due to underflow error, because we had made sure at this point that amountsIn[0] < msg.value.

Tools Used

VSCode, Manual review

Recommended Mitigation Steps

Change the last statement in the function as follows:

if (msg.value > amountsIn[0]) _safeTransferAVAX(_to, msg.value - amountsIn[0]);