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

0 stars 0 forks source link

Using a zero-address check as a proxy for enforcing one-time initialization is risky #68

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

0xRajeev

Vulnerability details

Impact

Using a zero-address check as a proxy for enforcing one-time initialization is risky if the variable is allowed to (accidentally/unintentionally) be reset to zero along some flow. High-severity exploits have happened because of this assumption.

Proof of Concept

https://github.com/code-423n4/2021-09-wildcredit/blob/c48235289a25b2134bb16530185483e8c85507f8/contracts/LendingPair.sol#L82

https://github.com/code-423n4/2021-09-wildcredit/blob/c48235289a25b2134bb16530185483e8c85507f8/contracts/LPTokenMaster.sol#L33-L41

Tools Used

Manual Analysis

Recommended Mitigation Steps

Recommend to instead use a separate initialized variable for enforcing one-time init like how it is done in LPTokenMaster. This is the best-practice and doesn’t cost much more gas except requiring a separate variable.

talegift commented 3 years ago

It might be a good practice. But the code as it stands doesn't permit a reset of any token back to 0. This case is impossible.

ghoul-sol commented 3 years ago

Best practices, non-critical