sherlock-audit / 2024-02-telcoin-platform-audit-update-judging

3 stars 1 forks source link

0xKartikgiri00 - Missing _disableInitializers() Call in `Stablecoin` constructor. leads to the calling of `initialize` function inside the implementation contract. #57

Closed sherlock-admin4 closed 8 months ago

sherlock-admin4 commented 8 months ago

0xKartikgiri00

medium

Missing _disableInitializers() Call in Stablecoin constructor. leads to the calling of initialize function inside the implementation contract.

Summary

The Stablecoin contract is not calling _disableInitializers() function in its constructor. This allows the initialize function to be called on the implementation contract every time a proxy is deployed, this leads to the reinitialization of state variables.

Vulnerability Detail

In upgradeable contracts the state is store inside the proxy contract not in the implementation contract so that's why it is important that constructor should call _disableInitializers() function so that the initialize function is not called in the implementation contract. However, _disableInitializers() function is not being called in the constructor of the Stablecoin contract, allowing the initialize function to be called in implementation contract in this case which is Stablecoin contract.

Impact

Allows the initialize function to be called in implementation contract, which leads reinitialization of state variables.

Proof Of Concept

The below script shows how the initialize function can be called inside the implementation contract.

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.24;

import {Script} from "forge-std/Script.sol";
import {console} from "forge-std/Test.sol";
import {Stablecoin} from "../src/stablecoin/Stablecoin.sol";

contract StableCoin is Script{

    address public Attacker = makeAddr(("attacker"));

    function run() external returns(Stablecoin, address){
    vm.startBroadcast(Attacker);

    Stablecoin StableCoinContract = new Stablecoin();
@>  StableCoinContract.initialize("Attack", "ATK", 18);
    vm.stopBroadcast();

    return (StableCoinContract, Attacker);
    }

}

Code Snippet

https://github.com/sherlock-audit/2024-02-telcoin-platform-audit-update/blob/main/telcoin-contracts/contracts/stablecoin/Stablecoin.sol#L47

Tool used

Manual Review, Foundry

Recommendation

Ensure that the _disableInitializers() function is called in the constructor of the Stablecoin contract. This will prevent the initialize function from being called on the implementation contract.

+  constructor() {
+      _disableInitializers();
+       }
sherlock-admin4 commented 8 months ago

2 comment(s) were left on this issue during the judging contest.

WangAudit commented:

initializer modifier restricts initializing more than once; front-running initializers is invalid under Sherlock's rules

takarez commented:

invalid

kartik-giri commented 8 months ago

2 comment(s) were left on this issue during the judging contest.

WangAudit commented:

initializer modifier restricts initializing more than once; front-running initializers is invalid under Sherlock's rules

takarez commented:

invalid

The initiailze() function can be called from implementation contract even when it has been deployed if we don't implement _disableInitializers().