ControllerV1.sol initialization function callable multiple times

[code423n4]

Handle

sirhashalot

Vulnerability details

Impact

The initialize() function in ControllerV1.sol has a comment of "This function is not supposed to call multiple times" but it has no checks that prevent it from being called multiple times. An initialization function should only be possible to call once.

One side effect of this is that the ControllerV1.sol initializer function can overwrite the values of several address state variables, the dexAggregator variable, and the oleWethDexData variable. This can result in unexpected edge cases, such as bypassing the requirement that the address values not be the zero address, which is a requirement when the addresses are set using the setter functions.

Proof of Concept

The initialize() function in ControllerV1.sol specifically states that it should not be called multiple times, but the code does not match the intent because there are no checks that prevent the initializer from being called twice or more. The initialize function can set addresses to the zero address, bypassing the requirement found in the setter functions for the lpoolImplementation address, openLev address, dexAggregator address, and others.

Recommended Mitigation Steps

Use the initializer modifier from OpenZeppelin or a similar piece of code to validate that the initializer() function is only callable once.

[ColaM12]

Duplicate to #67

[0xleastwood]

In favour of selecting https://github.com/code-423n4/2022-01-openleverage-findings/issues/242 but as this issue is from the same warden, I'll mark this as invalid.