sherlock-audit / 2023-12-ubiquity-judging

2 stars 2 forks source link

bareli - _newOwner can be equal to current owner. #179

Closed sherlock-admin closed 7 months ago

sherlock-admin commented 8 months ago

bareli

medium

_newOwner can be equal to current owner.

Summary

There is no check for whether the _newOwner is the same as the current owner. While not a security flaw per se, it's redundant to allow ownership transfer to the current owner and could result in unnecessary gas costs.

Vulnerability Detail

contract OwnershipFacet is IERC173 { /// @inheritdoc IERC173 function transferOwnership(address _newOwner) external override { require( (_newOwner != address(0)), "OwnershipFacet: New owner cannot be the zero address" ); LibDiamond.enforceIsContractOwner(); LibDiamond.setContractOwner(_newOwner); }

Impact

it's redundant to allow ownership transfer to the current owner and could result in unnecessary gas costs.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/facets/OwnershipFacet.sol#L8

Tool used

Manual Review

Recommendation

use a require statement so that _newOwner can not be equal to current owner.

sherlock-admin2 commented 7 months ago

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

Input validation

sherlock-admin2 commented 7 months ago

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

Input validation

nevillehuang commented 7 months ago

Invalid, this is purely a sanity check and would consitute admin input erorr not valid based on sherlock rules, see point 5.