_validateDataIsExpected() validation checks are open to manipulation

[code423n4]

Lines of code

https://github.com/code-423n4/2022-02-aave-lens/blob/main/contracts/core/modules/FeeModuleBase.sol#L41

Vulnerability details

Impact

In feeModuleBase.sol the _validateDataIsExpected() function checks if the decodedAmount is equal to the amount argument or if the decodedCurrency is equal to the currency argument. The use of "||" here is dangerous because only one of the conditions needs to be true for the validation to pass when both of them should be required to be true and if not the function should revert. The current check leaves the protocol open to attack.

Proof of Concept

https://github.com/code-423n4/2022-02-aave-lens/blob/main/contracts/core/modules/FeeModuleBase.sol#L41

Tools Used

Manual code review

Recommended Mitigation Steps

consider using the && operator instead of || for the validation checks in the _validateDataIsExpected() function.

[Zer0dot]

Again this is just wrong. It's a modern error not a require statement!

[0xleastwood]

This is just wrong, both cases need to be checked. If either decodedAmount or decodedCurrency is invalid, the function should revert.