Upgraded Q -> 2 from #1553 [1698130820647]

[c4-judge]

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

3- Funds not transferred to correct recipient in RdpxDecayingBonds.emergencyWithdraw : Risk : Low The function RdpxDecayingBonds.emergencyWithdraw is used by the admin to pull funds from the contract in case of emergency situation, the function allows the admin to specify a recipient to address as input to the function, this address should receive all the extracted funds (as explained in function Natspec comments). But due to an error in function code only the native token is transferred to to address and the other tokens are transferred to admin address.

This issue has a Low impact because there is no loss of funds and the tokens are transferred to admin which can later give them to the to address but it should be corrected.

Proof of Concept File: decaying-bonds/RdpxDecayingBonds.sol Line 89-107

function emergencyWithdraw( address[] calldata tokens, bool transferNative, address payable to, // @audit should be the recipient uint256 amount, uint256 gas ) external onlyRole(DEFAULT_ADMIN_ROLE) { _whenPaused(); if (transferNative) { (bool success, ) = to.call{value: amount, gas: gas}(""); require(success, "RdpxReserve: transfer failed"); } IERC20WithBurn token;

for (uint256 i = 0; i < tokens.length; i++) {
    token = IERC20WithBurn(tokens[i]);
    // @audit funds not transferred to `to` address 
    token.safeTransfer(msg.sender, token.balanceOf(address(this)));
}

} Mitigation Transfer all the funds to the correct recipient address to.

[c4-judge]

GalloDaSballo marked the issue as duplicate of #250

[GalloDaSballo]

Pasted the wrong description, the issue is: 2 Missing sync() after transferring funds to RdpxV2Core Low 2 M

[c4-judge]

GalloDaSballo marked the issue as satisfactory

[GalloDaSballo]

2- Missing sync() after transferring funds to RdpxV2Core : Risk : Low In the RdpxV2Core contract there a sync() function which allows the update of the state variables tracking the tokens balances (ETH, DPX,...) by setting them equal to the actual contract token balance (with the use of token.balanceOf(address(this))).

The issue is that there are some instances where this function must be called after transferring funds to RdpxV2Core but it is not which could result in a temporarily incorrect balance accounting.

Proof of Concept There 2 instances of this issue :

File: amo/UniV2LiquidityAmo.sol Line 160-178

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 );

// @audit did not call IRdpxV2Core(rdpxV2Core).sync()

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

} The _sendTokensToRdpxV2Core() function is used many times in the UniV2LiquidityAmo to transfer tokens to RdpxV2Core but it doesn't call the sync() function after doing so, we can see that a similar function is present in the other AMO contract UniV3LiquidityAmo._sendTokensToRdpxV2Core and this one does call the sync() function after the transfers.

File: amo/UniV3LiquidityAmo.sol Line 119-133

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);
}

} In the second instance the call to univ3_positions.collect will transfer tokens to the RdpxV2Core contract but after the loop there no call to the sync() function to update the tokens accounting in RdpxV2Core.

Mitigation Add the IRdpxV2Core(rdpxV2Core).sync() call to the instances aforementioned to make sure that the accounting is always correct after a transfer from the AMOs contracts.