Address.isContract() is not a reliable way of checking if the input is an EOA

[hats-bug-reporter]

Github username: --
Submission hash (on-chain): 0x5224612aa1dbc3efd9856dd3081df2c4289d2c038ab2958c552958a3c1549563
Severity: high severity

Description:

Impact

Detailed description of the impact of this finding.

The expectation of the noContract is controlled using ! access control with Address.isContract, then because isContract() gets the size of the code length of the account in question by relying on extcodesize/address.code.length, this means that the restriction can be bypassed when deploying a smart contract through the smart contract's constructor call.

VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f/packages/contracts/contracts/protocol/oracles/VMEXOracle.sol

Line 242 in fb396a3

if (!Address.isContract(c._curvePool)) {

Recommendation

Modify the code to require(msg.sender == tx.origin);

[ksyao2002]

Thanks for the information. This is not a vulnerability, as c._curvePool can only be set by VMEX global admins. This check is more of a sanity check on the behalf of the VMEX team, and no funds are at risk. The restriction bypassing is impossible for contracts already deployed.

[Nabeel-javaid]

Thanks for the information. This is not a vulnerability, as c._curvePool can only be set by VMEX global admins. This check is more of a sanity check on the behalf of the VMEX team, and no funds are at risk. The restriction bypassing is impossible for contracts already deployed.

it is impossible for the contracts that is alr deployed, but not for the one that are in the process of deployment

also I do think that it should be atleast L or Med