Attacker can drain the token from the user's account

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-core/src/PositionManager.sol#L388

Vulnerability details

Vulnerability details

Impact

There is a potential vulnerability if the increaseLPAllowance() function is not implemented safely and allows for arbitrary increases to the token allowance.

File: ajna-core/src/PositionManager.sol

        pool.increaseLPAllowance(owner, params_.indexes, lpAmounts);

If an attacker gains control of the spender address, they can use the approve() function to increase the allowance of the spender for the token to a very large value, which can potentially result in the token being drained from the user's account.

Tools Used

Manual Review

Recommended mitigation steps

Here's how you can modify the increaseLPAllowance() function by implementing safeIncreaseAllowance() from OpenZeppelin:

        function increaseLPAllowance(
                address spender_,
                uint256[] calldata indexes_,
                uint256[] calldata amounts_
            ) external override nonReentrant {
-                LPActions.increaseLPAllowance(
-                                _lpAllowances[msg.sender][spender_],
-                                spender_,
-                                indexes_,
-                                amounts_
-                            );
+                for (uint256 i = 0; i < indexes_.length; i++) {
+                    uint256 index = indexes_[i];
+                    uint256 amount = amounts_[i];

+                    // approve owner to take over the LP ownership (required for transferLP pool call)
+                    SafeERC20.safeIncreaseAllowance(
+                        lpTokens[index],
+                        spender_,
+                        amount
+                    );

+                    _lpAllowances[msg.sender][spender_][index] = _lpAllowances[msg.sender][spender_][index].add(amount);
+                }
            }

Assessed type

Access Control

[c4-judge]

Picodes marked the issue as unsatisfactory: Insufficient quality