sherlock-audit / 2023-04-hubble-exchange-judging

7 stars 6 forks source link

ubermensch - Misuse of Constructor in Upgradable Contract #171

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ubermensch

high

Misuse of Constructor in Upgradable Contract

Summary

The smart contract uses the constructor to initialize the clearing_house address in the context of an upgradable contract. This approach is problematic because the variable will be initialized in the context of the implementation, not the proxy.

Vulnerability Detail

In an upgradable contract, the constructor is only called when the contract is first created. Therefore, any initialization done in the constructorwill only affect the implementation contract, not the proxy contract. This means that the clearing_house address will not be correctly initialized in the proxy contract, leading to potential issues with the contract's functionality.

Impact

The impact of this issue is significant. If the clearing_house address is not correctly initialized in the proxy contract, it could lead to malfunctioning of the contract. This could potentially result in loss of funds or other unintended consequences.

Code Snippet

https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/AMM.sol#L107-L109 https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/orderbooks/OrderBook.sol#L60-L63 https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/orderbooks/OrderBookSigned.sol#L60-L62

Tool used

Manual Review

Recommendation

Instead of using the constructor to initialize the clearing_houseaddress, consider including it in the initialize function that can be called after the contract is deployed.