search

code-423n4 / 2022-05-factorydao-findings

1 stars 1 forks source link

SINGLE-STEP PROCESS FOR OWNERSHIP TRANSFER #230

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/VoterID.sol#L153 https://github.com/code-423n4/2022-05-factorydao/blob/db415804c06143d8af6880bc4cda7222e5463c0e/contracts/VoterID.sol#L162

Vulnerability details

SINGLE-STEP PROCESS FOR OWNERSHIP TRANSFER

In VoterID.sol, ownership transfer is subject to a single-step process and can result in the ownership to be passed mistakenly to a random address, which would prevent some functions to be called.

Impact

Medium

Proof Of Concept

VoterID.sol:153: _owner_ = newOwner;

There is no zero-address check. If the owner were to accidentally pass the zero address as a parameter (or any random address), the _owner_ would get updated with that new address, and setTokenURI would not be able to be called anymore.

Tools Used

Manual Analysis

Recommended Mitigation Steps

use a two-step address change where the new owner must call a function to 'accept' the ownership of the contract.

illuzen commented 2 years ago

Zero address checks out of scope and duplicate #68

gititGoro commented 2 years ago

Downgrading severity to QA as the ownership transfer may happen between different users.

JeeberC4 commented 2 years ago

Adding to warden's QA Report #224