sherlock-audit / 2024-06-leveraged-vaults-judging

9 stars 8 forks source link

unRekt - No checks present in the `getRewardSettings()` function of `VaultRewarderLib.sol` to ensure if the index `for` loop is going out of bound, can lead to potential DoS attacks. #128

Closed sherlock-admin2 closed 2 months ago

sherlock-admin2 commented 2 months ago

unRekt

Medium

No checks present in the getRewardSettings() function of VaultRewarderLib.sol to ensure if the index for loop is going out of bound, can lead to potential DoS attacks.

Summary

No checks present in the getRewardSettings() function of VaultRewarderLib.sol to ensure if the index for loop it can lead to data corruption and DoS attacks as the function may revert because of incorrect memory access

Vulnerability Detail

In the getRewardSettings() function the loop

for (uint256 i; i < v.length; i++) v[i] = store[i];

does not check for out of bound condition.

And the length of v can be different from store as : Value of v depends on VaultRewardState[](s.numRewardTokens) Value of store depends on VaultStorage.getVaultRewardState()

No checks are present to ensure if they have the same length. And as the loop runs till i<v.length and mapping store if updated on basis of that store[i]. If length of v < store : it will leave empty spots in the mapping. If length of v > store : it can lead to overflow or out of bound error.

Impact

No checks are present to ensure v and store have the same length as array checks for the element out of bounds

Code Snippet

https://github.com/sherlock-audit/2024-06-leveraged-vaults/blob/14d3eaf0445c251c52c86ce88a84a3f5b9dfad94/leveraged-vaults-private/contracts/vaults/common/VaultRewarderLib.sol#L25-L32

Tool used

Manual Review

Recommendation

Implement checks to ensure the lengths of v and store are same Implement upper bound for the loop.

sherlock-admin2 commented 2 months ago

1 comment(s) were left on this issue during the judging contest.

0xmystery commented:

It should be assumed that this will not extend to some length that is unreasonable since the method is controlled by the owner