code-423n4 / 2023-08-dopex-findings

3 stars 3 forks source link

Upgraded Q -> 2 from #425 [1698130939448] #2227

Closed c4-judge closed 10 months ago

c4-judge commented 10 months ago

Judge has assessed an item in Issue #425 as 2 risk. The relevant finding follows:

  1. UniLiquidityAmo contracts doesn’t synchronize reserve balances of RdpxV2Core in some cases Impact Developer from Dopex said that “we keep the balances to check the health of dpxEth”. I talk about this balances: https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L61

    /// @notice Array containg the reserve assets ReserveAsset[] public reserveAsset;

    /// @dev Struct containing the the rdpxV2Core reserve asset data struct ReserveAsset { address tokenAddress; uint256 tokenBalance; string tokenSymbol; } tokenBalance can differ from actual balance in this cases:

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L313-L322 function recoverERC20( address tokenAddress, uint256 tokenAmount ) external onlyRole(DEFAULT_ADMIN_ROLE) { // Can only be triggered by owner or governance, not custodian // Tokens are sent to the custodian, as a sort of safeguard TransferHelper.safeTransfer(tokenAddress, rdpxV2Core, tokenAmount);

emit RecoveredERC20(tokenAddress, tokenAmount);

} https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV3LiquidityAmo.sol#L119-L133 function collectFees() external onlyRole(DEFAULT_ADMIN_ROLE) { for (uint i = 0; i < positions_array.length; i++) { Position memory current_position = positions_array[i]; INonfungiblePositionManager.CollectParams memory collect_params = INonfungiblePositionManager.CollectParams( current_position.token_id, rdpxV2Core, type(uint128).max, type(uint128).max );

  // Send to custodian address
  univ3_positions.collect(collect_params);
}

} https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/amo/UniV2LiquidityAmo.sol#L160-L178 function _sendTokensToRdpxV2Core() internal { uint256 tokenABalance = IERC20WithBurn(addresses.tokenA).balanceOf( address(this) ); uint256 tokenBBalance = IERC20WithBurn(addresses.tokenB).balanceOf( address(this) ); // transfer token A and B from this contract to the rdpxV2Core IERC20WithBurn(addresses.tokenA).safeTransfer( addresses.rdpxV2Core, tokenABalance ); IERC20WithBurn(addresses.tokenB).safeTransfer( addresses.rdpxV2Core, tokenBBalance );

emit LogAssetsTransfered(msg.sender, tokenABalance, tokenBBalance);

} Recommended Mitigation Steps Call IRdpxV2Core(rdpxV2Core).sync() in all the referenced code blocks

c4-judge commented 10 months ago

GalloDaSballo marked the issue as duplicate of #250

c4-judge commented 10 months ago

GalloDaSballo marked the issue as satisfactory