CertiK $SAFEMOON

CertiK audit report on $Safemoon

Due to the CertiK audit of $Safemoon we have to change our $MNRY token contract.

Certik Audit: https://www.certik.org/projects/safemoon

$MNRT contract: https://github.com/moonery-io/moonery-contracts/blob/main/contracts/Moonery.sol

Findings

SSL-10 Privileged ownership

Description

The owner of contract Safemoon and $MNRY has the permission to:

  1. change the address that can receive LP tokens

  2. lock the contract

  3. exclude/include addresses from rewards/fees

  4. set taxFee, liquidityFee and _maxTxAmount

  5. enable swapAndLiquifyEnabled

without obtaining the consensus of the community

Recommendation

Renounce ownership when it is the right timing, or gradually migrate to a timelock plus multisig governing procedure and let the community monitor in respect of transparency considerations.

Alleviation

Use OpenZeppelin Ownable, removing _previousowner. We also transparant about above functions activities, see also Token & Liquidity Locks

SSL- | Possible to gain ownership after renouncing the contract ownership

Description

An owner is possible to gain ownership of the contract even if he calls function renounceOwnership torenounce the ownership. This can be achieved by performing the following operations:.

1. Call lock to lock the contract. The variable _previousOwner is set to the current owner.

2. Call unlock to unlock the contract.

3. Call renounceOwnership to leave the contract without an owner.

4. Call unlock to regain ownership.

Recommendation

We advise updating/removing lock and unlock functions in the contract; or removing the renounce Ownership if such a privilege retains at the protocol level. If timelock functionality could be introduced, we recommend using the implementation of Compound finance as reference. Reference:https://github.com/compound-finance/compound-protocol/blob/master/contracts/Timelock.sol

Removed implementation of Ownable.sol from $Safemoon and use import "@openzeppelin/contracts/access/Ownable.sol" instead. We can't introduce Timelock.sol due the max size of the contract.

Stuck ERC20 tokens inside $MNRY contracts

Description

see also: https://docs.google.com/document/d/1Feh5sP6oQL1-1NHi-X1dbgT3ch2WdhbXRevDN681Jv4/

If a user will make a mistake and choose a wrong function then the token will get stuck inside our contract (contract will not recognize a transaction). There will be no way to extract stuck tokens.

Solution

We have add fallbackRedeem new IERC20 token, with the ability to recover ERC20 tokens stuck in the contract. The function doesn't have the ability to recover $MNRY and Liquidity tokens $CAKE LP. We only send stick ERC20 token to our lottery contract and add this token as rewards for the community. newToken.safeTransfer(address(_lottery), tokenAmount);

    /**
     * @dev Fallback Redeem tokens. and send to lottery contract
     * Purpose, sending other ERC20 tokens to Lottery contract other than
     * $MNRY, $CAKE LP 
     *
     *
     * @param newToken_ Address of the token
     * @param tokenAmount_ Number of tokens to be emitted
    */
    function fallbackRedeem(IERC20 newToken_, uint256 tokenAmount_) external nonReentrant {
        require(!_isExcluded[_msgSender()], "Excluded addresses cannot call this function");
        require(address(newToken_) != address(this), "cannot recover to $MNRY");
        require(address(newToken_) != address(pancakePair), "cannot recover to $CAKE LP");
        newToken_.safeTransfer(address(_lottery), tokenAmount_);
    }

Max contract Size

Description

Due the above changes the Moonery contract exceed the size limit for testnet and mainnet deployment.

Solutions

Shorten error message

Last updated