MiguelContext
Most developers follow industry's standards to avoid re-creating the wheel and falling into common issues. Solidity world is not an exception of it.
One of the most popular standards is OpenZeppelin library which provides us several useful implementations such as ERC20 token, ERC721 token, Governor, etc.
Issue
But well, not everything in the standards must be used without taking into account the risks we could face in the future. As far as we know, in public blockchains like Ethereum this could potentially lead to hackers draining funds from users and protocols.
Let my bring up a method provided by OpenZeppelin which was discussed several times last year.
Address.isContract()
Explanation
This routine returns true in case the provided address contains some code which makes sense. Well, actually there are some special situations where the condition gets broken:
- Because during contract creation the code of the contract is equal to zero,
isContract()routine will return false in constructor scope. - If you send an address where a new contract will be located in the future.
- If you send an address where some contract was previously located and then deleted.
Risks
There is at least one common scenario where your protocol could get into trouble:
In case you just call some address assuming it is not a contract you could open a window for reentrancy attack.
So, how to correctly validate if an address is a contract?
Proposed WA
Actually it is a sort of limitation placed in the EVM. I guess it would have several ways to be solved but it is not known when a solution will take place.
Some time ago I faced a misusage of this function during an audition in Code4rena platform. For this issue I proposed a sort of public whitelist of the addresses where the final user could:
Add the address as a whitelisted EOA with the current blocknumber.
Before doing the stuff we have to:
- Validate the whitelisting.
- Validate that currentBlock is higher than the whitelisting block number.
- Finally, validate Address.isContract() method from OZ standard.
These steps are not the most user friendly flow but it works if you want to avoid calling some contract.
Next steps
As I mentioned at the beginning, this issue was discussed several times during last year resulting in the removal of isContract() method on OZ version 5.0. But be careful because this method will be marked as deprecated (but able to use) on previous versions.
https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3682
