sherlock-audit / 2023-04-splits-judging

4 stars 1 forks source link

ravikiran.web3 - CreateSwapper accepts address(0x0) as owner #4

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ravikiran.web3

high

CreateSwapper accepts address(0x0) as owner

Summary

SwapperFactory.createSwapper() function accepts address(0x0) as owner. After the owner is set as address(0x0), the ability to manage the swapper will be lost.

Vulnerability Detail

After the address(0x) is set as owner, the managing features for the swapper are lost in SwapperImpl contract.

Impact

With address(0x0) set as owner, the below functions are not accessible any more due to owner check.

a) setBeneficiary b) setTokenToBeneficiary c) setOracle

Code Snippet

https://github.com/0xSplits/splits-swapper/blob/83ce1124767a097aac37d1cd162a9b27bbc48701/src/SwapperFactory.sol#L40-L55

https://github.com/0xSplits/splits-swapper/blob/83ce1124767a097aac37d1cd162a9b27bbc48701/src/SwapperImpl.sol#L126-L136

Tool used

Manual Review

Recommendation

check in OwnableImpl.__initOwnable() where before setting, it should checks for incoming value to be a valid one before setting the owner in the storage. The function should revert incase address(0x0) was passed as owner