sherlock-audit / 2022-10-illuminate-judging

3 stars 0 forks source link

ak1 - Redeemer.sol#L168 : setFee never be called . #191

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ak1

high

Redeemer.sol#L168 : setFee never be called .

Summary

Redeemer.sol#L168 : setFee function is used to set the fee value.

But the problems is the function never pass and revert because of this if condition check, https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L170-L171

Vulnerability Detail

When we look at the feeChange, this can be observed in following line of codes.

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L59

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L169

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L184

The problems here is, the feeChange is nowhere assigned with valid value. That mean, it has default value zero.

looking at the setFee function's first few lines,

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L168-L172

The assigned feeTime will always be zero. So this will go to if check and revert.

Impact

setFee be never called. Fee can not be changed when required.

Code Snippet

https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Redeemer.sol#L168-L187

Tool used

Manual Review

Recommendation

I see the similar functionality in Lender.sol#L228. https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Lender.sol#L228

here, the feeChange can be set by admin by calling scheduleFeeChange https://github.com/sherlock-audit/2022-10-illuminate/blob/main/src/Lender.sol#L813-L820

It is recommended to follow similar approach for Redeemer also.

Duplicate of #34