code-423n4 / 2022-07-swivel-findings

0 stars 1 forks source link

Lack of zero-address input check at `setAdmin` function #144

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Swivel/Swivel.sol#L428-L432 https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Marketplace/MarketPlace.sol#L53-L56 https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Creator/Creator.sol#L47-L50

Vulnerability details

Impact

Accidentally setting the input parameter a to address(0) will lead to loosing control of the admin functionalities. In the event that this is accidentally set to address(0), no one then can change it again because only the admin(in this case address(0) ) can call this function which means no one. This is not good for the protocol because many important functions can only be called by the admin.

Proof of Concept

https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Swivel/Swivel.sol#L428-L432 https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Marketplace/MarketPlace.sol#L53-L56 https://github.com/code-423n4/2022-07-swivel/blob/daf72892d8a8d6eaa43b9e7d1924ccb0e612ee3c/Creator/Creator.sol#L47-L50

Tools Used

VIm

Recommended Mitigation Steps

I suggest adding a require statement, for example: require(a != address(0));

JTraversa commented 2 years ago

https://github.com/code-423n4/2022-07-swivel#input-sanitization

bghughes commented 2 years ago

https://github.com/code-423n4/2022-07-swivel#input-sanitization

Yes agreed, out of scope and informational at best. Downgrading to QA.

bghughes commented 2 years ago

Grouping this with the warden’s QA report, #141