santipu
    • Create new note
    • Create a note from template
      • Sharing URL Link copied
      • /edit
      • View mode
        • Edit mode
        • View mode
        • Book mode
        • Slide mode
        Edit mode View mode Book mode Slide mode
      • Customize slides
      • Note Permission
      • Read
        • Only me
        • Signed-in users
        • Everyone
        Only me Signed-in users Everyone
      • Write
        • Only me
        • Signed-in users
        • Everyone
        Only me Signed-in users Everyone
      • Engagement control Commenting, Suggest edit, Emoji Reply
    • Invite by email
      Invitee

      This note has no invitees

    • Publish Note

      Share your work with the world Congratulations! 🎉 Your note is out in the world Publish Note

      Your note will be visible on your profile and discoverable by anyone.
      Your note is now live.
      This note is visible on your profile and discoverable online.
      Everyone on the web can find and read all notes of this public team.
      See published notes
      Unpublish note
      Please check the box to agree to the Community Guidelines.
      View profile
    • Commenting
      Permission
      Disabled Forbidden Owners Signed-in users Everyone
    • Enable
    • Permission
      • Forbidden
      • Owners
      • Signed-in users
      • Everyone
    • Suggest edit
      Permission
      Disabled Forbidden Owners Signed-in users Everyone
    • Enable
    • Permission
      • Forbidden
      • Owners
      • Signed-in users
    • Emoji Reply
    • Enable
    • Versions and GitHub Sync
    • Note settings
    • Note Insights New
    • Engagement control
    • Make a copy
    • Transfer ownership
    • Delete this note
    • Save as template
    • Insert from template
    • Import from
      • Dropbox
      • Google Drive
      • Gist
      • Clipboard
    • Export to
      • Dropbox
      • Google Drive
      • Gist
    • Download
      • Markdown
      • HTML
      • Raw HTML
Menu Note settings Note Insights Versions and GitHub Sync Sharing URL Create Help
Create Create new note Create a note from template
Menu
Options
Engagement control Make a copy Transfer ownership Delete this note
Import from
Dropbox Google Drive Gist Clipboard
Export to
Dropbox Google Drive Gist
Download
Markdown HTML Raw HTML
Back
Sharing URL Link copied
/edit
View mode
  • Edit mode
  • View mode
  • Book mode
  • Slide mode
Edit mode View mode Book mode Slide mode
Customize slides
Note Permission
Read
Only me
  • Only me
  • Signed-in users
  • Everyone
Only me Signed-in users Everyone
Write
Only me
  • Only me
  • Signed-in users
  • Everyone
Only me Signed-in users Everyone
Engagement control Commenting, Suggest edit, Emoji Reply
  • Invite by email
    Invitee

    This note has no invitees

  • Publish Note

    Share your work with the world Congratulations! 🎉 Your note is out in the world Publish Note

    Your note will be visible on your profile and discoverable by anyone.
    Your note is now live.
    This note is visible on your profile and discoverable online.
    Everyone on the web can find and read all notes of this public team.
    See published notes
    Unpublish note
    Please check the box to agree to the Community Guidelines.
    View profile
    Engagement control
    Commenting
    Permission
    Disabled Forbidden Owners Signed-in users Everyone
    Enable
    Permission
    • Forbidden
    • Owners
    • Signed-in users
    • Everyone
    Suggest edit
    Permission
    Disabled Forbidden Owners Signed-in users Everyone
    Enable
    Permission
    • Forbidden
    • Owners
    • Signed-in users
    Emoji Reply
    Enable
    Import from Dropbox Google Drive Gist Clipboard
       Owned this note    Owned this note      
    Published Linked with GitHub
    • Any changes
      Be notified of any changes
    • Mention me
      Be notified of mention me
    • Unsubscribe
    ![](https://i.imgur.com/zGzXYfT.jpg) __Audit Firm__: Solidity Lab __Client Firm__: Poodl __Prepared By:__ Cryptor, Jonatas Martins, Santipu __Delivery Date:__ March 7th, 2022 <br /> __Poodl__ engaged Solidity Lab to review the security of its Smart Contract system. From __February 26th, 2022__ to __March 7th, 2022__, a team of __3__ auditors reviewed the source code in scope. All findings have been recorded in the following report. Notice that the examined smart contracts are not resistant to external/internal exploit. For a detailed understanding of risk severity, source code vulnerability, and potential attack vectors, refer to the complete audit report below. # Project Overview | Project Name | RaisinLabs | |--------------|----------------------------------------------------------------------------------------------------------| | Language | Solidity | | Codebase | [tokenAudit](https://github.com/poodlTech/tokenAudit/) | | Commit | [eebe26...9f2c84](https://github.com/poodlTech/tokenAudit/tree/eebe267b3fdd75a82e09cc270b3c046b2c9f2c84/) | | Delivery Date | March 7th, 2022 | |-------------------|--------------------------------| | Audit Methodology | Static Analysis, Manual Review | | Vulnerability Level | Total | Pending | Declined | Acknowledged | Partially Resolved | Resolved | |---------------------|-------|---------|----------|--------------|--------------------|----------| | [Critical](#Critical)| 1 | 1 | 0 | 0 | 0 | 0 | | [High](#High) | 4 | 4 | 0 | 0 | 0 | 0 | | [Medium](#Medium) | 5 | 5 | 0 | 0 | 0 | 0 | | [Low](#Low) | 30 | 30 | 0 | 0 | 0 | 0 | # Audit Scope & Methodology ## Scope | ID File | SHA-1 | Checksum | |---------|------------|---------------------------------------| | TK | Token.sol | 47242647ae17e7bcd48b9ba6839076ffaf7add3a25c448f43aa511a26fbf7318 | | DP | DividendPayingToken.sol|d4b180f6a62e550e17ac4ac5a3012dfbae85c02e8344c74ef44b3d4dfd21ab7d | | PR | Presale.sol| 08dc34b82c139f659f01f8433c5bdf3f377f7fca0807930822a9832233ed4bc2 | |AR | Airdropper.sol| fb586c32f5141845b6157869d944a37902565d92f1dbd6db4cdf0e0966475b5e | ## Methodology The auditing process pays special attention to the following considerations: - Testing the smart contracts against both common and uncommon attack vectors. - Assessing the codebase to ensure compliance with current best practices and industry standards. - Ensuring contract logic meets the specifications and intentions of the client. - Cross-referencing contract structure and implementation against similar smart contracts produced by industry leaders. - Thorough line-by-line manual review of the entire codebase by community auditors. ## Vulnerability Classifications | Vulnerability Level | Classification | |---------------------|----------------------------------------------------------------------------------------------| | [Critical](#Critical) | Easily exploitable by anyone, causing loss/manipulation of assets or data. | | [High](#High) | Arduously exploitable by a subset of addresses, causing loss/manipulation of assets or data. | | [Medium](#Medium) | Inherent risk of future exploits that may or may not impact the smart contract execution. | | [Low](#Low) | Minor deviation from best practices. | # Protocol Graph (Optional) IMAGE # Findings & Resolutions | ID | Title | Category | Severity | Status |-------|-----------|---------|----------|---------| [C-01](#C01) | Uninitialised variables leads to DoS in various functions | DoS| CRITICAL | Pending | [H-01](#H01) | Contract doesn't compile | Compilation| HIGH | Pending | [H-02](#H02) | Loss of all dividends when updating dividend tracker| Logic | HIGH | Pending | [H-03](#H03) | Unset `_marketingWalletAddress` leads to loss of funds |Initialization | HIGH | Pending | [H-04](#H04) | Slippage control is not correctly implemented leading to loss of funds | Logic | HIGH | Pending | [M-01](#M01) | Centralization Risk for trusted owners | Ownership | MEDIUM | Pending | [M-02](#M02) | Missing check for custom token | Validation | MEDIUM | Pending | [M-03](#M03) | Check from setSellTopUp can be circumvented | Validation | MEDIUM | Pending | [M-04](#M04) | No check in low-level call | Validation | MEDIUM | Pending | [M-05](#M05) | Excluding and including user from dividends removes him from holders list | Validation | MEDIUM | Pending | [L-01](#L01) | Unused Contracts | Unused Code | LOW | Pending | [L-02](#L02) | Unused Events | Unused Code | LOW | Pending | [L-03](#L03) | Trapped ETH | Logic | LOW | Pending | [L-04](#L04) | Unreachable code | Dead Code | LOW | Pending | [L-05](#L05) | Two storage mappings can be deleted | Gas Optimization | LOW | Pending | [L-06](#L06) | Unnecesary `_transfer` function | Unused Code | LOW | Pending | [L-07](#L07) | `withdrawDividend` function can be removed | Unused Code | LOW | Pending | [L-08](#L08) | Avoid use fixed gas fees | Gas Optimization | LOW | Pending | [L-09](#L09) | Missing event emission | Event | LOW | Pending | [L-10](#L10) | Use a more recent version of Solidity | Versioning | LOW | Pending | [L-11](#L11) | Consider Two-Step Ownership Transfer | Ownership | LOW | Pending | [L-12](#L12) | Multiple contracts in same file | Architecture | LOW | Pending | [L-13](#L13) | No need to use safeMath Library in solidity v0.8.4 | Versioning | LOW | Pending | [L-14](#L14) | Return values of `approve()` not checked | Validation | LOW | Pending | [L-15](#L15) | Unsafe ERC20 operations | Logic | LOW | Pending | [L-16](#L16) | Using bools for storage incurs overhead | Gas Optimization | LOW | Pending | [L-17](#L17) | Cache array length outside of loop | Gas Optimization | LOW | Pending | [L-18](#L18) | Use Custom Errors | Gas Optimization | LOW | Pending | [L-19](#L19) | Don’t initialize variables with default value | Gas Optimization | LOW | Pending | [L-20](#L20) | Long revert strings | Gas Optimization | LOW | Pending | [L-21](#L21) | `++i` costs less gas than `i++`, especially when it’s used in forloops (`--i`/`i--` too) | Gas Optimization | LOW | Pending | [L-22](#L22) | Splitting `require()` statements that use `&&` saves gas | Gas Optimization | LOW | Pending | [L-23](#L23) | Use `!= 0` instead of `> 0` for unsigned integer comparison | Gas Optimization | LOW | Pending | [L-24](#L24) | In constructor the `_marketingWalletAddress` is set to `address(0)` | Unused Code | LOW | Pending | [L-25](#L25) | Useless set of initial `totalFees` | Unused Code | LOW | Pending | [L-26](#L26) | New owner is not excluded from fees | Logic | LOW | Pending | [L-27](#L27) | No checks if the account is already excluded from fees or blacklisted | Validation | LOW | Pending | [L-28](#L28) | Old marketing wallets are excluded from fees | Logic | LOW | Pending | [L-29](#L29) | Unused declared variables | Unused Code | LOW | Pending | [L-30](#L30) | WalletFee should not be set when TotalFee = 0 | Logic | LOW | Pending | ## <a id="Critical"></a>Critical ### <a id="C01"></a> C-01 Uninitialised variables leads to DoS in various functions https://github.com/poodlTech/tokenAudit/blob/main/contracts/Airdropper.sol#L40 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Presale.sol#L38 #### Description: The `Airdropper` and `Presale` contracts have empty constructors so there're variables that aren't initialised. In `Airdropper`, the variable `newToken` is never initialised so the two most critical functions `airdrop` and `emergencyWithdraw` will always revert causing a DoS. In `Presale`, the variables `cap`, `token` and `exchangeRatio` are never initialised and that leads to a DoS in the functions `buyPresale`, `emergencyWithdraw` and `closeAirdrop`. #### Recommendation: Initialise all the mentioned variables in the constructor to avoid the `Airdropper` and `Presale` contracts to be unusable. ```diff - constructor() {} + constructor(address _newToken) { + newToken = _newToken; + } ``` ```diff - constructor() {} + constructor(uint256 _cap, address _token, uint256 _exchangeRatio) { + cap = _cap; + token = _token; + exchangeRatio = _exchangeRatio; + } ``` #### Resolution: ---- ### <a id="H01"></a> H-01 Contract doesn't compile https://github.com/poodlTech/tokenAudit/blob/main/contracts/Presale.sol#L51 #### Description: The`Presale` contract doesn't compile due to a missing `payable` modifier in the `buyPresale` function. #### Recommendation: Add a `payable` modifier to the `buyPresale` function. ```diff - function buyPresale(uint256 value) public { + function buyPresale(uint256 value) public payable { ``` #### Resolution: ### <a id="H02"></a> H-02 Loss of all dividends when updating dividend tracker https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L113-L125 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L534 #### Description: It's not possible to change the dividend tracker to a tracker that was already set before. So if a dividend tracker contains dividens that weren't sent to the user when the owner changes the dividend tracker all the dividends will be lost. If the owner tries to set a dividend tracker that was set before, the transaction will revert resulting in the loss of unclaimed dividends contained in that tracker. This happens because in `updateDividendTracker` there is a line that exclude the dividend tracker from dividends. Given that an address can't be excluded from dividends if it's already excluded, the transaction will revert. ```solidity function updateDividendTracker(address newAddress) public onlyOwner { require(newAddress != address(dividendTracker), ""); DividendTracker newDividendTracker = DividendTracker(payable(newAddress)); require(newDividendTracker.owner() == address(this), ""); // @audit the dividend tracker has to be excluded from dividends newDividendTracker.excludeFromDividends(address(newDividendTracker)); /* ... */ } ``` ```solidity function excludeFromDividends(address account) external onlyOwner { // @audit an account can't be exluded from dividends if it's already excluded require(!excludedFromDividends[account],""); excludedFromDividends[account] = true; _setBalance(account, 0); tokenHoldersMap.remove(account); emit ExcludeFromDividends(account); } ``` #### POC: ```solidity function testDividendTrackerChange() public { token = new Token(); DividendTracker tracker = new DividendTracker(); tracker.transferOwnership(address(token)); DividendTracker oldTracker = token.dividendTracker(); // @audit first time changing to a new dividend tracker token.updateDividendTracker(address(tracker)); // @audit if try to change back to oldTracker gets an error // all the funds in old tracker will be lost vm.expectRevert(); token.updateDividendTracker(address(oldTracker)); } ``` #### Recommendation: It is recommended to change the `excludeFromDividends` function to the following code: ```diff function excludeFromDividends(address account) external onlyOwner { - require(!excludedFromDividends[account],""); excludedFromDividends[account] = true; _setBalance(account, 0); tokenHoldersMap.remove(account); emit ExcludeFromDividends(account); } ``` If we remove that `require()`, a dividend tracker will be able to be set a second time and recover the unclaimed dividends. #### Resolution: ### <a id="H03"></a> H-03 Unset `_marketingWalletAddress` leads to loss of funds https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L373 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L414 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L420 #### Description: By default `_marketingWalletAddress` is `address(0)`, and this variable is used in some functions. These functions make low level call to `_marketingWalletAddress` and send some ether or transfer any tokens, if the marketing address is not set, it will lose all ETH or tokens. #### POC Consider the case that Token contract has some native tokens, the `_marketingWalletAddress` was forgot to be set and the owner wants to withdraw. By calling `withdrawBNB` function all native tokens will be transferred to `_marketingWalletAddress == address(0)` and it will be lost. #### Recommendation: It's is recommended to validate that the `_marketingWalletAddress` is different from `address(0)` before send any token. ```diff function withdrawBNB() public onlyOwner nonReentrant { + require(_marketingWalletAddress != address(0)); uint256 amountBNB = address(this).balance; (bool success, ) = payable(_marketingWalletAddress).call{value: amountBNB}(""); require(success, "transfer failed"); } ``` ```diff function withdrawBep20(address token) public onlyOwner nonReentrant{ + require(_marketingWalletAddress != address(0)); require((IERC20(address(token)).balanceOf(address(this)))>0); IERC20(token).safeTransfer(payable(_marketingWalletAddress),IERC20(token).balanceOf(address(this))); } ``` ```diff function swapBack(uint256 contractTokenBalance) internal { ... + require(_marketingWalletAddress != address(0)); (success,) = address(_marketingWalletAddress).call{value: amountBNBMarketing}(""); ... } ``` #### Resolution: ### <a id="H04"></a> H-04 Slippage control is not correctly implemented leading to loss of funds https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L388-L394 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L397-L409 #### Description: In function `swapTokensForEth` and `addLiquidity` there is no slippage protection, making it vulnerable to sanwich attacks, MEV exploits and may lead to significant loss of yield. #### Recommendation: There are different ways to set the slippage. The first one is to let users determine the maximum slippage they're willing to take. The protocol front-end should set the recommended value for them. The second one is have a slippage control parameters that's set by the owner. #### Resolution: ---------------- ## <a id="Medium"></a> Medium ### <a id="M01"></a> M-01 Centralization Risk for trusted owners All Contracts #### Description: Contracts have owners with privileged rights to perform admin tasks and need to be trusted to not perform malicious updates or drain funds, in this case the owner has too much power and can steal all assets #### Recommendation: It is recommended to use a multisig wallet as owner of the contracts. And even if a multisig wallet is used, it's encouraged to follow the principle of the least privilege. #### Resolution: ### <a id="M02"></a> M-02 Missing check for custom token https://github.com/poodlTech/tokenAudit/blob/main/contracts/DividendPayingToken.sol#L135 #### Description: In `_withdrawDividendOfUser` the contract checks if the user has a custom reward token before distributing the rewards but it doesn’t check if that custom token is still approved by the contract. In the docs it states _"Both tokens and AMMs need to be approved by the team"_. There’s a possibility that the contract first approve a custom token and then it revokes the approval. When the approval of a custom token is revoked, no users should be receiving the dividends in that token. #### Recommendation: In `_withdrawDividendOfUser` add a check to ensure that the custom token the user has is still approved. Original code: ```solidity if(!userHasCustomRewardToken[user]){ ... } ``` Risk mitigation: ```solidity // @audit If the user has a custom reward token but it's not approved now, // it should receive the dividends in BNB instead of the custom token if(!userHasCustomRewardToken[user] || !approvedTokens[userCurrentRewardToken[user]]){ ... } ``` #### Resolution: ### <a id="M03"></a> M-03 Check from setSellTopUp can be circumvented https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L217-L220 #### Description: In the `setSellTopUp` function, there’s a check that ensures that the `sellTopUp` fee plus `rewardsFee`, `marketingFee` and `liquidityFee` is never above `capFees`. But the problem arises that in the `setMarketingFee`, `setLiquidityFee` and `setRewardsFee` functions don’t check for the `sellTopUp` fee. So first we could set the `sellTopUp` to 15, and later set the other fee variables that’ll sum 15. So finally the `sellTopUp`, `rewardsFee`, `marketingFee` and `liquidityFee` will add 30. #### Recommendation: It is recommended to always validate if the fee are under the `capFees`, consider creating only one function to set all the fees. #### Resolution: ### <a id="M04"></a> M-04 No check in low-level call https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L369-L376 #### Description: In swapBack function there is 2 calls that send value to the dividendTracker and the marketingWalletAddress. In both calls there is no require statement that checks that the call is sent correctly. #### Recommendation: It is recommended to always check the return in low-level calls. #### Resolution: ### <a id="M05"></a> M-05 Excluding and including user from dividends removes him from holders list https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L533-L545 #### Description: By removing and user from dividends it is removed from `tokenHoldersMap`, if the same user is included again in dividends, he is not added to map. #### Recommendation: It is recommended to include again the user in `tokenHoldersMap` in `includeInDividends` function. #### Resolution: ----------------- ## <a id="Low"></a> Low ### <a id="L01"></a> L-01 Unused Contracts https://github.com/poodlTech/tokenAudit/blob/main/contracts/Airdropper.sol#L5 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Airdropper.sol#L7 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Airdropper.sol#L10-L24 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Presale.sol#L6 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Presale.sol#L7 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Presale.sol#L10-L24 #### Description: There are imported contracts that aren't used in the code. This happens in various instances. #### Recommendation: The imported contracts that aren't used should be removed from the code to reduce code size, avoid confusions and improve readability. #### Resolution: ### <a id="L02"></a> L-02 Unused Events https://github.com/poodlTech/tokenAudit/blob/main/contracts/Airdropper.sol#L35-L38 #### Description: There is an event in the `Airdropper` contract that is not emitted anywhere in the code. #### Recommendation: The events that aren't emitted in the contract should be removed to reduce code size, avoid confusions and improve readability. #### Resolution: ### <a id="L03"></a> L-03 Trapped ETH https://github.com/poodlTech/tokenAudit/blob/main/contracts/Airdropper.sol#L42 https://github.com/poodlTech/tokenAudit/blob/eebe267b3fdd75a82e09cc270b3c046b2c9f2c84/contracts/Token.sol#L108 #### Description: In the `Airdropper` contract there's an unnecessary `receive` function and no method to withdraw ETH, so if by error any user sends ETH to that contract it will be stuck there forever. Also in the Token contract, there is an unnecessary `receive` function that is empty and doesn't do anything. If a user sends ETH to this contract directly by mistake, they will be claimed by the owner. #### Recommendation: Remove the `receive` function in the `Airdropper` contract. Remove the `receive` function in the `token` contract. ### <a id="L04"></a> L-04 Unreachable code https://github.com/poodlTech/tokenAudit/blob/main/contracts/DividendPayingToken.sol#L191-L196 #### Description: In the `_transfer` function we have 3 lines of code that will never execute. These 3 lines are a copy-paste of the `_transfer` function of the original ERC1726. #### Recommendation: The unreachable code should be removed to avoid confusions and improve readability. #### Resolution: ### <a id="L05"></a> L-05 Two storage mappings can be deleted https://github.com/poodlTech/tokenAudit/blob/main/contracts/DividendPayingToken.sol#L48 https://github.com/poodlTech/tokenAudit/blob/main/contracts/DividendPayingToken.sol#L51 #### Description: The mappings `userHasCustomRewardAMM` and `userHasCustomRewardToken` can be deleted because the mappings `userCurrentRewardToken` and `userCurrentRewardAMM` can do the same check. To know if the user has a custom reward token or AMM the contract can check if the value of `userCurrentRewardToken` and `userCurrentRewardAMM` is address zero or not. #### Recommendation: The mappings `userHasCustomRewardAMM` and `userHasCustomRewardToken` can be deleted to save a lot of gas and the checks done by this mappings can be perfomed by the `userCurrentRewardToken` and `userCurrentRewardAMM` mappings. #### Resolution: ### <a id="L06"></a> L-06 Unnecesary `_transfer` function https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L524-L527 #### Description: The `_transfer` function in the `DividendTracker` contract always `revert()` when called it overrides the `_transfer` function in `DividendPayingToken` that also always `revert()`. ```solidity // DividendTracker#524 function _transfer(address, address, uint256) internal override pure { //Users can not transfer this token as it has to be 1:1 with the parent token. require(false, ""); } ``` The `_transfer` function above overrides the `_transfer` function below. ```solidity // DividendPayingToken#191 function _transfer(address from, address to, uint256 value) internal virtual override { require(false); int256 _magCorrection = magnifiedDividendPerShare.mul(value).toInt256Safe(); magnifiedDividendCorrections[from] = magnifiedDividendCorrections[from].add(_magCorrection); magnifiedDividendCorrections[to] = magnifiedDividendCorrections[to].sub(_magCorrection); } ``` #### Recommendation: This `_transfer` function in `DividendTracker` can be removed to reduce code size, improve readability and avoid confusions. #### Resolution: ### <a id="L07"></a> L-07 `withdrawDividend` function can be removed https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L529 #### Description: The `withdrawDividend` function in `DividendTracker` overrides `withdrawDividend.` in `DividendPayingToken`. The first function always reverts but the second one can be called from the `DividendPayingToken` contract directly and use it. So the first function can be removed as it’s useless. ```solidity // DividendTracker#529 function withdrawDividend() public override pure { require(false, "Use the 'claim' function"); } ``` The function above overrides the function below. The first one can be removed. ```solidity // DividendPayingToken#125 function withdrawDividend() external virtual override { _withdrawDividendOfUser(payable(msg.sender)); } ``` #### Recommendation: This `withdrawDividend` function in `DividendTracker` can be removed to reduce code size, improve readability and avoid confusions. #### Resolution: ### <a id="L08"></a> L-08 Avoid use fixed gas fees https://github.com/poodlTech/tokenAudit/blob/main/contracts/DividendPayingToken.sol#L53 #### Description: The `stipend` variable stores the gas fee to spend in the call to withdraw the dividends of the users. This is a bad idea because in times of high volatility the gas fees can increase to a point where the `stipend` is not enough to cover the gas in order to withdraw the dividends of the users and transactions could revert. #### Recommendation: Remove the `stipend` variable and opt by not limiting the gas sent to a call. If the `stipend` variable is used to avoid reentrancy, it's a better practice using a reentrancy guard. #### Resolution: ### <a id="L09"></a> L-09 Missing event emission https://github.com/poodlTech/tokenAudit/blob/main/contracts/Airdropper.sol#L47-L56 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Presale.sol#L51-L58 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Presale.sol#L60-L65 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Presale.sol#L72-L75 #### Description: There are state-changing methods that should emit some events so that off-chain monitoring can be implemented. #### Recommendation: Make sure to emit a proper event in each state-changing method to follow best practices. #### Resolution: ### <a id="L10"></a> L-10 Use a more recent version of Solidity All contracts #### Description: For security, it is best practice to use the latest solidity version. For the security fix list in the versions; https://github.com/ethereum/solidity/blob/develop/Changelog.md #### Recommendation: It is recommended to update solidity version to recent #### Resolution: ### <a id="L11"></a> L-11 Consider Two-Step Ownership Transfer All contracts #### Description: Owner can call the `transferOwnership` function to transfer the ownership to a new address directly. As such, there is a risk that the ownership is transferred to an invalid address, thus causing the contract to be without a owner. #### Recommendation: Implement a two-step ownership transfer approach by using `Ownable2Step` from OpenZeppelin as opposed to `Ownable` as it gives you the security of not unintentionally sending the owner role to an address you do not control. #### Resolution: ### <a id="L12"></a> L-12 Multiple contracts in same file All contracts #### Description: Here is the list of instance of multiple contract declaration in same folder: - Declaration of `Reentrancy` abstract contract in multiple files - DividendTracker is declared inside `Token.sol` #### Recommendation: Do not create same contract in different files or multiple contracts in same file, you should create one contract per file. #### Resolution: ### <a id="L13"></a> L-13 No need to use safeMath Library in solidity v0.8.4 All contracts #### Description: The safemath Library is normally used to prevent underflow and overflows when uint variable is out of range. However, solidity V0.8.4 already checks for underflow and overflow so there is no need to use SafeMath. #### Recommendation: Remove SafeMath Library #### Resolution: ### <a id="L14"></a> L-14 Return values of `approve()` not checked https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L385 #### Description: Not all IERC20 implementations revert when there’s a failure in `approve()`. The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually approving anything #### Recommendation: It is recommended to check the return value of approve. #### Resolution: ### <a id="L15"></a> L-15 Unsafe ERC20 operations https://github.com/poodlTech/tokenAudit/blob/main/contracts/Airdropper.sol#L67 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Presale.sol#L56 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Presale.sol#L68 #### Description: There are many Weird `ERC20` Tokens that won't work correctly using the standard `IERC20` interface. For example, `IERC20(token).transferFrom()` and `IERC20(token).transfer()` will fail for some tokens as they may not conform to the standard `IERC20` interface. #### Recommendation: Consider using `SafeERC20` for `transferFrom`, `transfer` and `approve`. #### Resolution: ### <a id="L16"></a> L-16 Using bools for storage incurs overhead https://github.com/poodlTech/tokenAudit/blob/main/contracts/Airdropper.sol#L32 https://github.com/poodlTech/tokenAudit/blob/main/contracts/DividendPayingToken.sol#L48-L49 https://github.com/poodlTech/tokenAudit/blob/main/contracts/DividendPayingToken.sol#L51-L52 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Presale.sol#L36 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L38 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L44 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L56-L57 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L512 #### Description: Use `uint256(1)` and `uint256(2)` for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from `false` to `true`, after having been `true` in the past. See source. #### Recommendation: It is recommended to change from bool to uint256 in mappings and state variables. #### Resolution: ### <a id="L17"></a> L-17 Cache array length outside of loop https://github.com/poodlTech/tokenAudit/blob/main/contracts/Airdropper.sol#L49 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Airdropper.sol#L60 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L159 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L170 #### Description: If not cached, the solidity compiler will always read the length of the array during each iteration. That is, if it is a storage array, this is an extra sload operation (100 additional extra gas for each iteration except for the first) and if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first). #### Recommendation: It is recommended to cache the array length outisde of loop #### Resolution: ### <a id="L18"></a> L-18 Use Custom Errors https://github.com/poodlTech/tokenAudit/blob/main/contracts/Airdropper.sol#L19 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Airdropper.sol#L51 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Presale.sol#L41 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L421 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L530 #### Description: If not cached, the solidity compiler will always read the length of the array during each iteration. That is, if it is a storage array, this is an extra sload operation (100 additional extra gas for each iteration except for the first) and if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first). #### Recommendation: Source Instead of using error strings, to reduce deployment and runtime cost, you should use Custom Errors. This would save both deployment and runtime cost. #### Resolution: It is recommended to use custom errors, as this example: ```solidity //Declare the error error Unauthorized(); //Use the error revert Unauthorized(); ``` ### <a id="L19"></a> L-19 Don’t initialize variables with default value https://github.com/poodlTech/tokenAudit/blob/main/contracts/Airdropper.sol#L49 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Airdropper.sol#L60 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L159 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L170 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L574 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L576-L577 #### Description: It is not necessary to initialize variables with default values #### Recommendation: It is recommended to remove the initialization for default values ```diff - uint256 value = 0 + uint256 value; - bool flag = false + bool flag; ``` #### Resolution: ### <a id="L20"></a> L-20 Long revert strings https://github.com/poodlTech/tokenAudit/blob/main/contracts/Airdropper.sol#L51 #### Description: Using string with more than 32 bytes consume more gas #### Recommendation: It is recommended to change the revert error string to less than 32bytes or create custom errors. #### Resolution: ### <a id="L21"></a> L-21 `++i` costs less gas than `i++`, especially when it’s used in forloops (`--i`/`i--` too) https://github.com/poodlTech/tokenAudit/blob/main/contracts/Airdropper.sol#L49 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Airdropper.sol#L60 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L159 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L170 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L580 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L587 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L590 #### Description: `++i` can save 5 gas per loop. #### Recommendation: It is recommended to change from i++ to ++i and to save more gas you can use the `unchecked` block. #### Resolution: ### <a id="L22"></a> L-22 Splitting `require()` statements that use `&&` saves gas https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L297 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L548 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L338 #### Description: Require statements that use `&&` consume more gas than multiple require statements. #### Recommendation: It is recommended to split the require statements #### Resolution: ### <a id="L23"></a> L-23 Use `!= 0` instead of `> 0` for unsigned integer comparison https://github.com/poodlTech/tokenAudit/blob/main/contracts/DividendPayingToken.sol#L114-L115 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L374 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L413 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L603 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L657 #### Description: The `!= 0` consumes less gas than `> 0` #### Recommendation: It is recommended to use `!= 0` instead of `> 0` #### Resolution: ### <a id="L24"></a> L-24 In constructor the `_marketingWalletAddress` is set to address(0) https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L98 #### Description: The line 98, `excludeFromFees(_marketingWalletAddress, true)` is setting exclude from fees for address(0) #### Recommendation: It is recommended to either set `_marketingWalletAddress` in constructor before exclude from fees or remove this line. #### Resolution: ### <a id="L25"></a> L-25 Useless set of initial `totalFees` https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L52 #### Description: At the start of the contract, the variable totalFees is set to equal `rewardsFee + liquidityFee + marketingFee`.This calculation is not needed because the initial values of this variables are all zero. #### Recommendation: It is recommended to either remove this useless calculation or set the initial fees on the constructor. #### Resolution: ### <a id="L26"></a> L-26 New owner is not excluded from fees https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol #### Description: In the constructor, the owner of the contract is excluded from fees but if the ownership is transferrred, the new owner won’t be excluded from fees. #### Recommendation: Override `transferOwnership` function to exclude the new owner from fees #### Resolution: ### <a id="L27"></a> L-27 No checks if the account is already excluded from fees or blacklisted https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L158-L163 https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L169-L173 #### Description: In the `excludeFromFees` function, it checks if the account to be excluded from the fees is already excluded. This is okey but in the `excludeMultipleAccountsFromFees` the check is missing. Also, the `blacklistAddress` and `blacklistMultipleAddresses` functions don’t have checks to ensure the account to be blacklisted is already blacklisted. #### Recommendation: It is recommended to check if the account is already excluded from the fee in `excludeMultipleAccountsFromFees` and check if address is already blacklisted in `blacklistMultipleAddresses` #### Resolution: ### <a id="L28"></a> L-28 Old marketing wallets are excluded from fees https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L187-192 #### Description: In the `setMarketingWallet` function, it excludes from fees to the new marketing wallet but the old marketing wallet will still be excluded from fees. #### Recommendation: It is recommended to change the `setMarketingWallet` function, set that the old marketing wallet is now not excluded from fees. #### Resolution: ### <a id="L29"></a> L-29 Unused declared variables https://github.com/poodlTech/tokenAudit/blob/main/contracts/Token.sol#L355-357 #### Description: The variable `path` is not used, it is redeclared inside `swapTokensForEth` #### Recommendation: It is recommended to remove this dead code to save gas #### Resolution: ### <a id="L30"></a> L-30 Check should be made that Owner cannot set WalletFee is TotalFees = 0 https://github.com/poodlTech/tokenAudit/blob/eebe267b3fdd75a82e09cc270b3c046b2c9f2c84/contracts/Token.sol#L337-L342 #### Description: If TotalFee = 0, then the function transfer will not incur any walletfees even if it is > 0 #### Recommendation: Check should be made that Owner cannot set WalletFee is TotalFees = 0 #### Resolution: ____ ## Disclaimer > > This report is not, nor should be considered, an “endorsement” or “disapproval” of any particular project or team. This report is not, nor should be considered, an indication of the economics or value of any “product” or “asset” created by any team or project that contracts Solidity Lab to perform a security assessment. This report does not provide any warranty or guarantee regarding the absolute bug-free nature of the technology analyzed, nor do they provide any indication of the technologies proprietors, business, business model or legal compliance. > > This report should not be used in any way to make decisions around investment or involvement with any particular project. This report in no way provides investment advice, nor should be leveraged as investment advice of any sort. This report represents an extensive assessing process intending to help our customers increase the quality of their code while reducing the high level of risk presented by cryptographic tokens and blockchain technology. > > Blockchain technology and cryptographic assets present a high level of ongoing risk. Solidity Lab’s position is that each company and individual are responsible for their own due diligence and continuous security. Solidity Lab’s goal is to help reduce the attack vectors and the high level of variance associated with utilizing new and consistently changing technologies, and in no way claims any guarantee of security or functionality of the technology we agree to analyze. > > The assessment services provided by Solidity Lab is subject to dependencies and under continuing > development. You agree that your access and/or use, including but not limited to any services, reports, and materials, will be at your sole risk on an as-is, where-is, and as-available basis. Cryptographic tokens are emergent technologies and carry with them high levels of technical risk and uncertainty. The assessment reports could include false positives, false negatives, and other unpredictable results. The services may access, and depend upon, multiple layers of third-parties. > > Notice that smart contracts deployed on the blockchain are not resistant from internal/external exploit. Notice that active smart contract owner privileges constitute an elevated impact to any smart contract’s safety and security. Therefore, Solidity Lab does not guarantee the explicit security of the audited smart contract, regardless of the verdict. <br/> ____ ## About Solidity Lab is a community of practicing auditors guided by Guardian Audits. To learn more, visit https://lab.guardianaudits.com To view the Solidity Lab audit portfolio, visit https://github.com/GuardianAudits/LabAudits

    Import from clipboard

    Paste your markdown or webpage here...

    Advanced permission required

    Your current role can only read. Ask the system administrator to acquire write and comment permission.

    This team is disabled

    Sorry, this team is disabled. You can't edit this note.

    This note is locked

    Sorry, only owner can edit this note.

    Reach the limit

    Sorry, you've reached the max length this note can be.
    Please reduce the content or divide it to more notes, thank you!

    Import from Gist

    Import from Snippet

    or

    Export to Snippet

    Are you sure?

    Do you really want to delete this note?
    All users will lose their connection.

    Create a note from template

    Create a note from template

    Oops...
    This template has been removed or transferred.
    Upgrade
    All
    • All
    • Team
    No template.

    Create a template

    Upgrade

    Delete template

    Do you really want to delete this template?
    Turn this template into a regular note and keep its content, versions, and comments.

    This page need refresh

    You have an incompatible client version.
    Refresh to update.
    New version available!
    See releases notes here
    Refresh to enjoy new features.
    Your user state has changed.
    Refresh to load new user state.

    Sign in

    Forgot password

    or

    By clicking below, you agree to our terms of service.

    Sign in via Facebook Sign in via Twitter Sign in via GitHub Sign in via Dropbox Sign in with Wallet
    Wallet ( )
    Connect another wallet

    New to HackMD? Sign up

    Help

    • English
    • 中文
    • Français
    • Deutsch
    • 日本語
    • Español
    • Català
    • Ελληνικά
    • Português
    • italiano
    • Türkçe
    • Русский
    • Nederlands
    • hrvatski jezik
    • język polski
    • Українська
    • हिन्दी
    • svenska
    • Esperanto
    • dansk

    Documents

    Help & Tutorial

    How to use Book mode

    Slide Example

    API Docs

    Edit in VSCode

    Install browser extension

    Contacts

    Feedback

    Discord

    Send us email

    Resources

    Releases

    Pricing

    Blog

    Policy

    Terms

    Privacy

    Cheatsheet

    Syntax Example Reference
    # Header Header 基本排版
    - Unordered List
    • Unordered List
    1. Ordered List
    1. Ordered List
    - [ ] Todo List
    • Todo List
    > Blockquote
    Blockquote
    **Bold font** Bold font
    *Italics font* Italics font
    ~~Strikethrough~~ Strikethrough
    19^th^ 19th
    H~2~O H2O
    ++Inserted text++ Inserted text
    ==Marked text== Marked text
    [link text](https:// "title") Link
    ![image alt](https:// "title") Image
    `Code` Code 在筆記中貼入程式碼
    ```javascript
    var i = 0;
    ```
    var i = 0;
    :smile: :smile: Emoji list
    {%youtube youtube_id %} Externals
    $L^aT_eX$ LaTeX
    :::info
    This is a alert area.
    :::

    This is a alert area.

    Versions and GitHub Sync
    Get Full History Access

    • Edit version name
    • Delete

    revision author avatar     named on  

    More Less

    Note content is identical to the latest version.
    Compare
      Choose a version
      No search result
      Version not found
    Sign in to link this note to GitHub
    Learn more
    This note is not linked with GitHub
     

    Feedback

    Submission failed, please try again

    Thanks for your support.

    On a scale of 0-10, how likely is it that you would recommend HackMD to your friends, family or business associates?

    Please give us some advice and help us improve HackMD.

     

    Thanks for your feedback

    Remove version name

    Do you want to remove this version name and description?

    Transfer ownership

    Transfer to
      Warning: is a public team. If you transfer note to this team, everyone on the web can find and read this note.

        Link with GitHub

        Please authorize HackMD on GitHub
        • Please sign in to GitHub and install the HackMD app on your GitHub repo.
        • HackMD links with GitHub through a GitHub App. You can choose which repo to install our App.
        Learn more  Sign in to GitHub

        Push the note to GitHub Push to GitHub Pull a file from GitHub

          Authorize again
         

        Choose which file to push to

        Select repo
        Refresh Authorize more repos
        Select branch
        Select file
        Select branch
        Choose version(s) to push
        • Save a new version and push
        • Choose from existing versions
        Include title and tags
        Available push count

        Pull from GitHub

         
        File from GitHub
        File from HackMD

        GitHub Link Settings

        File linked

        Linked by
        File path
        Last synced branch
        Available push count

        Danger Zone

        Unlink
        You will no longer receive notification when GitHub file changes after unlink.

        Syncing

        Push failed

        Push successfully