Rewards can be stolen from LiquidityMiningPath.sol

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-canto/blob/main/canto_ambient/contracts/callpaths/LiquidityMiningPath.sol#L65-L81

Vulnerability details

Impact

First problem The absence of governance checks in the setConcRewards and setAmbRewards functions poses a significant security risk, allowing unauthorized entities to manipulate rewards for specific pools within defined time ranges. The impact includes the potential for financial losses to users and the creation of imbalances within the system. Malicious actors could exploit this vulnerability to tamper with reward settings, undermining the integrity of the smart contract.

Second problem The problem is that neither of the contracts possesses the required functionality to set governance

Proof of Concept

function setConcRewards(bytes32 poolIdx, uint32 weekFrom, uint32 weekTo, uint64 weeklyReward) public payable {
        // require(msg.sender == governance_, "Only callable by governance");
        require(weekFrom % WEEK == 0 && weekTo % WEEK == 0, "Invalid weeks");
        while (weekFrom <= weekTo) {
            concRewardPerWeek_[poolIdx][weekFrom] = weeklyReward;
            weekFrom += uint32(WEEK);
        }
    }

    function setAmbRewards(bytes32 poolIdx, uint32 weekFrom, uint32 weekTo, uint64 weeklyReward) public payable {
        // require(msg.sender == governance_, "Only callable by governance");
        require(weekFrom % WEEK == 0 && weekTo % WEEK == 0, "Invalid weeks");
        while (weekFrom <= weekTo) {
            ambRewardPerWeek_[poolIdx][weekFrom] = weeklyReward;
            weekFrom += uint32(WEEK);
        }
    }

As you can see, 'require' statements are commented. Anyone can call these functions and set rewards in a way that benefits malicious users.

Also, if we look at LiquidityMiningPath.sol and all contracts that this contract inherits from, we can notice that there is no way to set _governance

Tools Used

Manual Review

Recommended Mitigation Steps

Add a constructor where _governance will be set, and uncomment the require statements

Assessed type

Access Control

[c4-pre-sort]

141345 marked the issue as duplicate of #4

[c4-pre-sort]

141345 marked the issue as sufficient quality report

[c4-judge]

dmvt marked the issue as unsatisfactory: Invalid