Greetings y'all ! :wave:
I have the following concern in regards to the ownership and role transfer of several of your contracts and a suggestion to it:
Currently, whenever an ownership of a contract is changed, the process is as follows: The caller provides a new address to become the owner of the contract. Then a check if the caller is the current owner of that contract is performed. If it passes, the ownership of the contract is changed to be the provided address. Which seems okay and works flawlessly, but my concern is that there is still a chance for the current owner to provide a malicious address or just to make an unintentional mistake by providing false address to the function. This will be very upsetting especially if it happens to some of the core contracts in AAVE.
I would rate this with possible/unlikely likelihood and high severity
One suggestion is to make a 2 step verification/authentication. It would look something like similar to the already existing method for transferring the ownership, with the exception of that the provided address in the function is set to be as a lets say pendingOwner role(could be just an address saved in the contract). And then, a separate call, that actually has to be performed only by the pendingOwner address. So basically, in the end, the second call only checks if the caller is the pendingOwner address, and if it is - set the caller/pendingOwner as the new owner of the contract.
Setting a wrong address as a owner of a contract can be crucial and is for sure an undesirable behaviour!
milad237
commented
2 years ago
Greetings y'all ! :wave: I have the following concern in regards to the ownership and role transfer of several of your contracts and a suggestion to it:
Currently, whenever an ownership of a contract is changed, the process is as follows: The
callerprovides a newaddressto become the owner of the contract. Then a check if thecalleris the currentownerof that contract is performed. If it passes, the ownership of the contract is changed to be the providedaddress. Which seems okay and works flawlessly, but my concern is that there is still a chance for the currentownerto provide a maliciousaddressor just to make an unintentional mistake by providing falseaddressto the function. This will be very upsetting especially if it happens to some of the core contracts in AAVE.I would rate this with possible/unlikely likelihood and high severity
One suggestion is to make a 2 step verification/authentication. It would look something like similar to the already existing method for transferring the ownership, with the exception of that the provided
addressin the function is set to be as a lets say pendingOwner role(could be just an address saved in the contract). And then, a separate call, that actually has to be performed only by the pendingOwner address. So basically, in the end, the second call only checks if thecalleris thependingOwneraddress, and if it is - set thecaller/pendingOwneras the newownerof the contract.Setting a wrong
addressas a owner of a contract can be crucial and is for sure an undesirable behaviour!