CertiK $SAFEMOON
CertiK audit report on $Safemoon
Due to the CertiK audit of $Safemoon we have to change our $MNRY token contract.
ID | Category | Severity | Location | Changes |
SSL-01 Incorrect error message | Logical Issue | 3 | Moonery.sol:109 | The message "Account is already excluded" is changed to "Account is not excluded" . |
SSL-01 Incorrect error message | Logical Issue | 3 | Moonery.sol:118 | The message "Account is already excluded" is changed to "Account is not excluded" . |
SSL-02 Redundant code | Logical Issue | 🔵 | Moonery.sol: 452 | Code has been removed |
SSL-03 | Logical Issue | - | - | BNB in $MNRY contract automatically distribute to our holders by claimBNBReward function |
SSL-04 Centralized risk in addLiquidity | Centralization / Privilege | 🔴 | Moonery.sol:601 | owner() changed to address(this) |
SSL-05 Variable could be declared as constant | Gas Optimization | 🔵 | Moonery.sol:35 | changed to: uint256 private constant _tTotal = 1000000000 10 ** 6 10 ** 9; |
SSL-05 Variable could be declared as constant | Gas Optimization | 🔵 | Moonery.sol:39 | changed to: string private constant _name = "Moonery"; |
SSL-05 Variable could be declared as constant | Gas Optimization | 🔵 | Moonery.sol:40 | change to: string private constant _symbol = "MNRY"; |
SSL-05 Variable could be declared as constant | Gas Optimization | 🔵 | Moonery.sol:41 | change to: uint8 private immutable _decimals = 9; |
SSL-06 Return value not handled | Volatile Code | 🔵 | MooneryUtils.sol:105 | Handle return value from Pancakeswap. public returns (uint amountToken, uint amountETH, uint liquidity) |
SSL-07 rd party dependencies | Contr ol Flow | 3 | - | Changes on 3rd party library means changes on $MNRY contract, we are aware of this. |
SSL-08 Missing event emitting | Coding Style | 🔵 | | |
SSL-09 Function and variable naming doesn't match the operating environment | Coding Style | 🔵 | MooneryUtils.sol:54 | Function changed to swapTokensForBnb |
SSL-09 Function and variable naming doesn't match the operating environment | Coding Style | 🔵 | MooneryUtils.sol:75 | Function change to swapBNBForTokens |
SSL-09 Function and variable naming doesn't match the operating environment | Coding Style | 🔵 | several locations | ethAmount changed to bnbAmount |
SSL-09 Function and variable naming doesn't match the operating environment | Coding Style | 🔵 | several locations | etheReceived change to bnbReceived |
SSL-10 Privileged ownership | Centralization / Privilege | 3 | see chapter SSL-10 | |
SSL-11 Typos in the contract | Coding Style | 🔵 | Moonery.sol:57 | changed to tokensIntoLiquidity |
SSL-11 Typos in the contract | Coding Style | 🔵 | | |
SSL-12 The purpose of function deliver | Control Flow | 🔵 | Moonery.sol:227 | require(!_isExcluded[sender], "Excluded addresses cannot call this function"); |
SSL- | Possible to gain ownership after renouncing the contract ownership | Logical Issue, Centralization / Privilege | 3 | Ownable | see chapter SSL--- |
| | | | |
| | | | |
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
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.
Use OpenZeppelin Ownable, removing _previousowner. We also transparant about above functions activities, see also Token & Liquidity Locks
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.
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.
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.
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_);
}
Due the above changes the Moonery contract exceed the size limit for testnet and mainnet deployment.
Shorten error message
Error | Meaning |
FB1 | fallbackRedeem: cannot recover to $MNRY |
FB2 | fallbackRedeem: cannot recover to $CAKE LP |
AC1 | Moonery: Account is not excluded |
AC2 | Excluded addresses cannot call this function |
BEP1 | BEP20: transfer amount exceeds allowance |
BEP2 | BEP20: decreased allowance below zero |
BEP3 | BEP20: approve from the zero address |
BEP4 | BEP20: approve to the zero address |
BEP5 | BEP20: transfer from the zero address |
BEP6 | BEP20: transfer to the zero address |
BEP7 | BEP20: transfer to the zero address |
RFT1 | Moonery: reflectionFromToken Amount must be less than supply |
TFR1 | Moonery: tokenFromReflection Amount must be less than total reflections |
| |
Last modified 2yr ago