First Flight #3: Thunder Loan

Summary

The executeOperation function in the MockFlashLoanReceiver contract has a check that if the initiator is not the owner of the receiver contract, executeOperation will revert. initiator is a parameter from the flashloan function in the ThunderLoan contract, and it is msg.sender. Msg.sender for an external call (like the function call that calls executeOperation) is the contract making the external call, which is the ThunderLoan contract. But s_owner is whoever deploys the MockFlashLoanReceiver contract, so this check will fail unless the ThunderLoan contract deploys the mock flash loan receiver contract. And the current code in the ThunderLoan contract does not deploy the flash loan receiver.

Also, this check is ineffective because anyone could call executeOperation and just fill in the address for s_owner in the initiator parameter, so the check isn't preventing people from trying to exploit the contract anyway.

Vulnerability Details

The initiator is msg.sender (of the external function call) per the flash loan function (fourth parameter):

Impact

This is an unnecessary check that doesn't add any extra security, and its configuration doesn't make sense given that ThunderLoan doesn't seem to own MockFlashLoanReceiver.

Tools Used

Manual review

Recommendations

Remove this check. There is already a check that the caller of executeOperation is ThunderLoan.