Trader Joe v2 contest - catchup's results

Lines of code

https://github.com/code-423n4/2022-10-traderjoe/blob/main/src/LBFactory.sol#L472-L481 https://github.com/code-423n4/2022-10-traderjoe/blob/main/src/LBFactory.sol#L412-L464

Vulnerability details

Impact

The owner can set the base fee and flashloan fee. However, there are no upper or lower limits when setting the fees. Therefore, owner can set a very high fee intentionally or by mistake, which would effect the users of the protocol.

412:     /// @notice Function to set the fee parameter of a LBPair
413:     /// @param _tokenX The address of the first token
414:     /// @param _tokenY The address of the second token
415:     /// @param _binStep The bin step in basis point, used to calculate log(1 + binStep)
416:     /// @param _baseFactor The base factor, used to calculate the base fee, baseFee = baseFactor * binStep
417:     /// @param _filterPeriod The period where the accumulator value is untouched, prevent spam
418:     /// @param _decayPeriod The period where the accumulator value is halved
419:     /// @param _reductionFactor The reduction factor, used to calculate the reduction of the accumulator
420:     /// @param _variableFeeControl The variable fee control, used to control the variable fee, can be 0 to disable them
421:     /// @param _protocolShare The share of the fees received by the protocol
422:     /// @param _maxVolatilityAccumulated The max value of volatility accumulated
423:     function setFeesParametersOnPair(
424:         IERC20 _tokenX,
425:         IERC20 _tokenY,
426:         uint16 _binStep,
427:         uint16 _baseFactor,
428:         uint16 _filterPeriod,
429:         uint16 _decayPeriod,
430:         uint16 _reductionFactor,
431:         uint24 _variableFeeControl,
432:         uint16 _protocolShare,
433:         uint24 _maxVolatilityAccumulated
434:     ) external override onlyOwner {
435:         ILBPair _LBPair = _getLBPairInformation(_tokenX, _tokenY, _binStep).LBPair;
436: 
437:         if (address(_LBPair) == address(0)) revert LBFactory__LBPairNotCreated(_tokenX, _tokenY, _binStep);
438: 
439:         bytes32 _packedFeeParameters = _getPackedFeeParameters(
440:             _binStep,
441:             _baseFactor,
442:             _filterPeriod,
443:             _decayPeriod,
444:             _reductionFactor,
445:             _variableFeeControl,
446:             _protocolShare,
447:             _maxVolatilityAccumulated
448:         );
449: 
450:         _LBPair.setFeesParameters(_packedFeeParameters);
451: 
452:         emit FeeParametersSet(
453:             msg.sender,
454:             _LBPair,
455:             _binStep,
456:             _baseFactor,
457:             _filterPeriod,
458:             _decayPeriod,
459:             _reductionFactor,
460:             _variableFeeControl,
461:             _protocolShare,
462:             _maxVolatilityAccumulated
463:         );
464:     }
465: 

471: 
472:     /// @notice Function to set the flash loan fee
473:     /// @param _flashLoanFee The value of the fee for flash loan
474:     function setFlashLoanFee(uint256 _flashLoanFee) external override onlyOwner {
475:         uint256 _oldFlashLoanFee = flashLoanFee;
476: 
477:         if (_oldFlashLoanFee == _flashLoanFee) revert LBFactory__SameFlashLoanFee(_flashLoanFee);
478: 
479:         flashLoanFee = _flashLoanFee;
480:         emit FlashLoanFeeSet(_oldFlashLoanFee, _flashLoanFee);
481:     }

Proof of Concept

https://github.com/code-423n4/2022-06-infinity-findings/issues/153 https://github.com/code-423n4/2022-03-biconomy-findings/issues/73

Tools Used

Manual review

Recommended Mitigation Steps

Include checks to limit the min-max values to be set for the fees