Basket.sol#changePublisher() Insufficient input validation

[code423n4]

Handle

WatchPug

Vulnerability details

As per the test, changePublisher to the current publisher should not be allowed:

https://github.com/code-423n4/2021-10-defiprotocol/blob/7ca848f2779e2e64ed0b4756c02f0137ecd73e50/contracts/test/Basket.test.js#L122-L122

let publisher = await basket.publisher();
expect(publisher).to.equal(addr2.address);

await expect(basket.connect(addr2).changePublisher(addr2.address)).to.be.reverted;

However, there is no such check to make sure that.

https://github.com/code-423n4/2021-10-defiprotocol/blob/7ca848f2779e2e64ed0b4756c02f0137ecd73e50/contracts/contracts/Basket.sol#L147-L148

function changePublisher(address newPublisher) onlyPublisher public override {
    require(newPublisher != address(0));

Recommendation

Change to:

function changePublisher(address newPublisher) onlyPublisher public override {
    require(newPublisher != address(0) && newPublisher != publisher);

[GalloDaSballo]

Agree with finding, because there's tets, hence proof that this is the desired functionality, the warden has proven that the code doesn't behave as the sponsor intended. Agree with the severity