code-423n4 / 2021-09-wildcredit-findings

0 stars 0 forks source link

Clone-and-own approach used for OZ libraries is susceptible to errors and missing upstream bug fixes #77

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

0xRajeev

Vulnerability details

Impact

Cloning issues: Copy-pasting code from libraries such as OpenZeppelin may result in incorrect code semantics for the context being copied to, copy over any vulnerabilities or miss any security fixes (or gas optimizations) applied to the original code. All these may lead to security issues over time.

While Ownable has been improved to add a 2-step transferOwnership and removal of renounceOwnership, others such as Address, Clones and Math libraries are also from OZ, apparently making some minor changes like deleting comments, reformatting etc.

Proof of Concept

Reference: See similar Medium-severity finding M09 in OpenZeppelin's Audit of HoldeFi: https://blog.openzeppelin.com/holdefi-audit

https://github.com/code-423n4/2021-09-wildcredit/blob/main/contracts/external/Address.sol

https://github.com/code-423n4/2021-09-wildcredit/blob/main/contracts/external/Clones.sol

https://github.com/code-423n4/2021-09-wildcredit/blob/main/contracts/external/Math.sol

Tools Used

Manual Analysis

Recommended Mitigation Steps

Consider directly importing OpenZeppelin contracts instead of reimplementing or copying them, unless there are major improvements made such as the 2-step transfer of ownership in Ownable.

talegift commented 2 years ago

As mentioned in the last audit by a judge.

I don't see what is the exploit here. Invalid.

https://github.com/code-423n4/2021-07-wildcredit-findings/issues/83

ghoul-sol commented 2 years ago

I'll keep this as best practices recommendation this time, non-critical.