onlyEOAEx() modifier can be bypassed

Summary

onlyEOAEx() modifier which is supposed to verify whether a user is a contract or an EOA will give wrong results

Vulnerability Detail

The modifier uses OZ's isContract() function. However, there are numeral cases in which the function will give wrong results, leading to unexpected behavior.

* Among others, `isContract` will return false for the following
     * types of addresses:
     *
     *  - an externally-owned account
     *  - a contract in construction
     *  - an address where a contract will be created
     *  - an address where a contract lived, but was destroyed
     *
     * Furthermore, `isContract` will also return true if the target contract within
     * the same transaction is already scheduled for destruction by `SELFDESTRUCT`,
     * which only has an effect at the end of a transaction.
     * ====
     *

Furthermore, preventing calls from contracts is highly discouraged. It breaks composability, breaks support for smart wallet like Gnosis Safe, and does not provide security since it can be circumvented by calling from a contract constructor/ selfdestruct.

Impact

Wrongfully assume user is an EOA

Code Snippet

https://github.com/sherlock-audit/2023-04-blueberry/blob/main/blueberry-core/contracts/BlueBerryBank.sol#L73-#L79 https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/master/contracts/utils/AddressUpgradeable.sol#L18-#L30

Tool used

Manual Review

Recommendation

Overall remove the only EOA modifier as it doesn't improve the security of the contract, but rather limits it

sherlock-audit / 2023-04-blueberry-judging

deadrxsezzz - onlyEOAEx() modifier can be bypassed #87