Submitted by Dravee, also found by bitbopper, hansfriese, saian, Tutturu, JMukesh, __141345__, neumo, parashar, Randyyy, phaze, hxzy, Lambda, cccz, SEVEN, ne0n, 8olidity, and RaoulSchaffranek
https://github.com/code-423n4/2022-10-traderjoe/blob/79f25d48b907f9d0379dd803fc2abc9c5f57db93/src/LBToken.sol#L182
https://github.com/code-423n4/2022-10-traderjoe/blob/79f25d48b907f9d0379dd803fc2abc9c5f57db93/src/LBToken.sol#L187
https://github.com/code-423n4/2022-10-traderjoe/blob/79f25d48b907f9d0379dd803fc2abc9c5f57db93/src/LBToken.sol#L189-L192
Using temporary variables to update balances is a dangerous construction that has led to several hacks in the past. Here, we can see that _toBalance can overwrite _fromBalance:
Furthermore, the safeTransferFrom function has the checkApproval modifier which passes without any limit if _owner == _spender :
Add the following test to LBToken.t.sol (run it with forge test --match-path test/LBToken.t.sol --match-test testSafeTransferFromOneself -vvvv):
As we can see here, this test checks that transfering all your funds to yourself doubles your balance, and its passing. This can be repeated again and again to increase your balance.
0x0Louis (Trader Joe) confirmed
Alex the Entreprenerd (judge) commented:
