Unsafe check in _migrateKernel function

[code423n4]

Lines of code

https://github.com/code-423n4/2022-08-olympus/blob/main/src/Kernel.sol#L348-L359

Vulnerability details

Impact

If a single module is breached and behaves in a way that wasn't expected, the whole kernel migration can be made impossible with no way to fix it.

This is extraordinarily important not to propagate bugs on higher levels of the protocol down to the lower levels, especially kernel.

Proof of Concept

Once a module makes a delegatecall and then/or selfdestructs, the call will be impossible (will revert because of a standard existence check made by Solidity on external calls on high level). If a module implemented changeKernel function in an improper way, it reverts or charges too much gas to fit in the max execution cost (block gas limit), this will cause the code linked to revert, DoSing all migration abilities.

Recommended Mitigation Steps

Ignore the existence of the contract by making the call on a lower level. Ignore the success of the changeKernel call. Limit gas cost to a reasonable number.

[ind-igo]

I am not sure I understand this one. Why would there would be a different implementation of changeKernel? That function is inherited by all policies and modules and is not overridable.

[0xLienid]

@ind-igo I suppose if someone wrote a contract abiding by the Module interface without inheriting Module and thus KernelAdapter it's maybe possible? Not sure though. It feels like a very extreme edge case, if even possible at all, that there's not a good solution for

[ind-igo]

I am marking this as disputed. Would be up to governance and the DAO to prevent something like this. Nothing to be done in the code, as the issue lacks detail and the recommended steps are very unclear.

[0xean]

closing as invalid. It sounds like basically an input validation issue, but agree it is too unclear to add any value.