c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-10-canto/blob/main/canto_ambient/contracts/mixins/LiquidityMining.sol#L286 https://github.com/code-423n4/2023-10-canto/blob/main/canto_ambient/contracts/mixins/LiquidityMining.sol#L193

Vulnerability details

Impact

The identified security concern in the smart contract code involves the potential for balance theft due to arbitrary code execution when sending rewards to the owner address. If the owner address is a smart contract and its receive function includes the selfdestruct operation, executing the claimAmbientRewards function can lead to the unintended self-destruction of the smart contract and the loss of its Ether balance. This can have significant repercussions, including the destruction of smart contracts and the disruption of the intended behavior of the system.

Proof of Concept

function claimAmbientRewards(
    address owner,
    bytes32 poolIdx,
    uint32[] memory weeksToClaim
) internal {
    ..
    if (rewardsToSend > 0) {
        (bool sent, ) = owner.call{value: rewardsToSend}("");
        require(sent, "Sending rewards failed");
    }
}

it s called here

function claimAmbientRewards(bytes32 poolIdx, uint32[] memory weeksToClaim) public payable {
    claimAmbientRewards(payable(msg.sender), poolIdx, weeksToClaim);
}

and the same bug exist in

function claimConcentratedRewards(
    address payable owner,
    bytes32 poolIdx,
    int24 lowerTick,
    int24 upperTick,
    uint32[] memory weeksToClaim
) internal {
    ..
    if (rewardsToSend > 0) {
        (bool sent, ) = owner.call{value: rewardsToSend}("");
        require(sent, "Sending rewards failed");
    }
}

and it s called in

function claimConcentratedRewards(bytes32 poolIdx, int24 lowerTick, int24 upperTick, uint32[] memory weeksToClaim)
    public
    payable
{
    claimConcentratedRewards(payable(msg.sender), poolIdx, lowerTick, upperTick, weeksToClaim);
}

Malicious smart contract can contain selfdestruct() in its receive function when executor calls one of the functions that invoke callOutAndBridgeMultiple like:

contract SelfDestructingContract {
public address owner;

constructor(address _owner ) {
owner = _owner
}

// All this does is self destruct and send funds to "to"
receive() external payable {
selfdestruct(payable(owner));
}

}

Tools Used

Manual review

Recommended Mitigation Steps

Consider using a pull payment pattern where the recipient (e.g., refundee) initiates the transfer of tokens or ETH instead of being sent tokens directly. This gives the recipient more control over the process and reduces the risk of malicious actions.

Assessed type

call/delegatecall

c4-pre-sort commented 1 year ago

141345 marked the issue as low quality report

141345 commented 1 year ago

invalid

no loss

c4-judge commented 1 year ago

dmvt marked the issue as unsatisfactory: Insufficient quality