sherlock-audit / 2023-12-dodo-gsp-judging

6 stars 5 forks source link

bareli - Access control::init function doesnot have access control #115

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 6 months ago

bareli

medium

Access control::init function doesnot have access control

Summary

The init function does not have access control, meaning that any user could potentially call it and initialize the contract. This could be a security risk if the contract is deployed without proper initialization. It should be protected by access control mechanisms, such as the Ownable pattern or similar.

Vulnerability Detail

Anyone can call init function and initialize the function variable.It should be protected by access control mechanisms, such as the Ownable pattern or similar.

Impact

Any one can call the init function and assign the variable this is not good.

Code Snippet

https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSP.sol#L34 function init( address maintainer, address baseTokenAddress, address quoteTokenAddress, uint256 lpFeeRate, uint256 mtFeeRate, uint256 i, uint256 k, bool isOpenTWAP ) external { // GSP can only be initialized once require(!_GSPINITIALIZED, "GSP_INITIALIZED"); // _GSPINITIALIZED is set to true after initialization _GSPINITIALIZED = true; // baseTokenAddress and quoteTokenAddress should not be the same require(baseTokenAddress != quoteTokenAddress, "BASE_QUOTE_CAN_NOT_BE_SAME"); // _BASETOKEN and _QUOTETOKEN should be valid ERC20 tokens _BASETOKEN = IERC20(baseTokenAddress); _QUOTETOKEN = IERC20(quoteTokenAddress);

    // i should be greater than 0 and less than 10**36
    require(i > 0 && i <= 10**36);
    _I_ = i;
    // k should be greater than 0 and less than 10**18
    require(k <= 10**18);
    _K_ = k;

    // _LP_FEE_RATE_ is set when initialization
    _LP_FEE_RATE_ = lpFeeRate;
    // _MT_FEE_RATE_ is set when initialization
    _MT_FEE_RATE_ = mtFeeRate;
    // _MAINTAINER_ is set when initialization, the address receives the fee
    _MAINTAINER_ = maintainer;
    _IS_OPEN_TWAP_ = isOpenTWAP;
    // if _IS_OPEN_TWAP_ is true, _BLOCK_TIMESTAMP_LAST_ is set to the current block timestamp
    if (isOpenTWAP) _BLOCK_TIMESTAMP_LAST_ = uint32(block.timestamp % 2**32);

    string memory connect = "_";
    string memory suffix = "GSP";
    // name of the shares is the combination of suffix, connect and string of the GSP
    name = string(abi.encodePacked(suffix, connect, addressToShortString(address(this))));
    // symbol of the shares is GLP
    symbol = "GLP";
    // decimals of the shares is the same as the base token decimals
    decimals = IERC20Metadata(baseTokenAddress).decimals();

    // ============================== Permit ====================================
    uint256 chainId;
    assembly {
        chainId := chainid()
    }
    // DOMAIN_SEPARATOR is used for approve by signature
    DOMAIN_SEPARATOR = keccak256(
        abi.encode(
            // keccak256('EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)'),
            0x8b73c3c69bb8fe3d512ecc4cf759cc79239f7b179b0ffacaa9a75d522b39400f,
            keccak256(bytes(name)),
            keccak256(bytes("1")),
            chainId,
            address(this)
        )
    );
    // ==========================================================================
}

Tool used

Manual Review

Recommendation

use some t should be protected by access control mechanisms, such as the Ownable pattern or similar.

Duplicate of #27