`setRewardWeight()` does not implement upper weight limit

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0xcd5008b750b6134ec0fb6eac4a720da2671a4a05285ee4e734d1a8884a2b36c0 Severity: low

Description: Affected code https://github.com/Cvg-Finance/hats-audit/blob/b3c66b1323cd555f5fa784b12736fd9b8f9ddfc5/contracts/Staking/Convex/cvxAsset/CvxAssetStakerBuffer.sol#L239-L243

Description\ In CvxAssetStakerBuffer.sol, setRewardWeight() allows to set the reward weight and this function can be accessed by contract owner.

    function setRewardWeight(uint256 weight) external onlyOwner {
        cvxAssetWrapper.setRewardWeight(weight);
    }

This function Natspec states, weight of the reward in % (max 10_000) It means that weight as input argument should not be greater than 10_000, However, the current implemented function does not ensure this restriction and allows to pass the value upto max uint256 value.

Impact max weight restriction is not implemented in setRewardWeight() so it allows the function caller to bypass it. This deviates the design from Natspec and violates it, Therefore, setRewardWeight() should impose the max weight validation while calling it.

Per contest rules, This issue is truely fits in low severity.

Issues where the behavior of the contracts differs from the intended behavior (as described in the docs and by common sense), but no funds are at risk.

Recommendation to fix\ Consider below changes.

    function setRewardWeight(uint256 weight) external onlyOwner {
+      require(weight <= 10_000, "invalid weight");
        cvxAssetWrapper.setRewardWeight(weight);
    }

[Jonsey]

No POC, if you supplied a POC you would see cvxAssetWrapper.setRewardWeight checks the upper limit

[0xRizwan]

@PlamenTSV Not sure, why its invalid.

Can you please link cvxAssetWrapper.setRewardWeight here. Tried finding it without success, i believe cvxAssetWrapper contract is OOS so not aware of its implementation.

[PlamenTSV]

@0xRizwan Hi, I am looking at an example wrapper inside the repo at: https://github.com/Cvg-Finance/hats-audit/blob/b3c66b1323cd555f5fa784b12736fd9b8f9ddfc5/CONVEX/CvxCrvStakingWrapper.sol#L1389-L1406 Which internally does an upper check limit. This is reason 1. The second reason is that the function is managed by the owner so it is a centralisation risk. And the third reason it that the wrapper is OOS like you mentioned.