Two-step change of a critical parameter

[code423n4]

Handle

pauliax

Vulnerability details

Impact

In Treasury function setAdmin allows an admin to change it to a different address. This function has no validations, even a simple check for zero-address is missing, and there is no validation of the new address being correct. If the admin accidentally uses an invalid address for which they do not have the private key, then the system gets locked because the swivel cannot be corrected and none of the other functions that require admin caller can be executed. A similar issue was reported in a previous contest and was assigned a severity of medium: code-423n4/2021-06-realitycards-findings#105

Recommended Mitigation Steps

Consider either introducing a two-step process or making a test call to the new admin before updating it.

[GalloDaSballo]

Agree with finding, for critical parameters, while it may seem trivial, it's important to have sanity checks to avoid permanent bricking, loss of funds or unintended consequences

[GalloDaSballo]

While I can agree with the past, and I was going to leave this as medium as it was acknowledged by the sponsor, I don't want to set the precedent that "forgetting to validate an address = an exploit".

Having checks for correctness is a good idea, a best practice, and a way to ensure guarantees to users, however it's not the same as having logical errors nor programming a system that can be exploited given specific circumnstances