--- title: StarLink CertiK Final Report For StarLink tags: final-report --- {%hackmd XdIXzOf5Ty2M3Uj1taFHIg %} <center> <img src="https://www.wing.vc/uploads/images/companies/certik-logo04.png" height="232" /> </center> <p style="font-size: 28px">StarLink Miner Protocol</p> <p style="font-size: 22px">Security Assessment</p> <p style="font-size: 18px">January 18th, 2021</p> For : StarLink By : Owan Li @ CertiK guilong.li@certik.org Bryan Xu @ CertiK buyun.xu@certik.org --- {%hackmd vb2ypisZSneY5y8y5ou-nw %} --- ## <img src="https://svgshare.com/i/Pp1.svg" width="40"/> Overview #### Project Summary <table> <tr> <td width="50%" valign="top"><b>Project Name</b></td> <td width="50%" valign="top"><a href="https://github.com/starlink-so/starlinkminer/tree/master/contracts">StarLink Miner</a></td> </tr> <tr> <td width="50%" valign="top"><b>Description</b></td> <td width="50%" valign="top">A defi miner protocol with SLN token</td> </tr> <tr> <td width="50%" valign="top"><b>Platform</b></td> <td width="50%" valign="top">Ethereum; Solidity; Yul</td> </tr> <tr> <td width="50%" valign="top"><b>Codebase</b></td> <td width="50%" valign="top"><a href="https://github.com/starlink-so/starlinkminer/tree/master/contracts">GitHub Repository</a></td> </tr> <tr> <td width="50%" valign="top"><b>Commit</b></td> <td width="50%" valign="top"> <a href="https://github.com/starlink-so/starlinkminer/commit/42e49305dad56c373b7b3602ed7c4a858b0022ea"> 42e49305dad56c373b7b3602ed7c4a858b0022ea</a><br/> <a href="https://github.com/starlink-so/starlinkminer/commit/ac56f1dfb4fe37c7af01cbd50fd6b0cf9e0edf2a"> ac56f1dfb4fe37c7af01cbd50fd6b0cf9e0edf2a</a><br/> <a href="https://github.com/starlink-so/starlinkminer/commit/3c30da5a5e5062d375262c2b3947cf9a354b571c"> 3c30da5a5e5062d375262c2b3947cf9a354b571c</a><br/> </td> </tr> </table> #### Audit Summary <table> <tr> <td width="50%" valign="top"><b>Delivery Date</b></td> <td width="50%" valign="top">Jan. 9th, 2021</td> </tr> <tr> <td width="50%" valign="top"><b>Method of Audit</b></td> <td width="50%" valign="top">Static Analysis, Manual Review</td> </tr> <tr> <td width="50%" valign="top"><b>Consultants Engaged</b></td> <td width="50%" valign="top">2</td> </tr> <tr> <td width="50%" valign="top"><b>Timeline</b></td> <td width="50%" valign="top">Jan. 5, 2021 - Jan. 7, 2021</td> </tr> </table> #### Vulnerability Summary <table> <tr> <td width="50%" valign="top"><b>Total Issues</b></td> <td width="50%" valign="top">12</td> </tr> <tr> <td width="50%" valign="top"><b>Total Critical</b></td> <td width="50%" valign="top">0</td> </tr> <tr> <td width="50%" valign="top"><b>Total Major</b></td> <td width="50%" valign="top">0</td> </tr> <tr> <td width="50%" valign="top"><b>Total Minor</b></td> <td width="50%" valign="top">3</td> </tr> <tr> <td width="50%" valign="top"><b>Total Informational</b></td> <td width="50%" valign="top">9</td> </tr> </table> --- ## <img src="https://svgshare.com/i/Pp1.svg" width="40"/> Executive Summary This report has been prepared for **Starlinkminer** smart contract to discover issues and vulnerabilities in the source code of their Smart Contract as well as any contract dependencies that were not part of an officially recognized library. A comprehensive examination has been performed, utilizing Dynamic Analysis, Static Analysis, and Manual Review techniques. 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 industry experts. --- ## <img src="https://svgshare.com/i/Pp1.svg" width="40"/> File in Scope | Contract | SHA-256 Checksum | | ----------------------------- | ------------------------------------------------------------ | | **SLNToken.sol** | d637293315c9b65c6cb6f81dfc727ee89b9cc28a7a13e94201a59a05a594b333 | | **StarPools.sol** | 803815efc095e2742ab2e9f17c98b3a21b16710888b4f88090bd5b0f2a950279 | | **WordFund.sol** | 251f7748b74dc3f801fd648fa50108174fe6a0f00844c373806922890e5df438 | | **DebugLogs.sol** | 7bbc56d4820e3341fc3490b45a43759c0049253f57e6ba990b568cc31af5f3af | --- ## <img src="https://svgshare.com/i/Pp1.svg" width="40"/> Documentation The sources of truth regarding the operation of the contracts in scope were lackluster and are something we advise to be enriched to aid in the legibility of the codebase as well as project. To help aid our understanding of each contract’s functionality we referred to in-line comments and naming conventions. These were considered the specification, and when discrepancies arose with the actual code behaviour, we consulted with the **StarLink** team or reported an issue. --- ## <img src="https://svgshare.com/i/Pp1.svg" width="40"/> Review Notes Certain optimization steps that we pinpointed in the source code mostly referred to coding standards and inefficiencies, however 3 minor vulnerabilities were identified during our audit that solely concerns the specification. Certain discrepancies between the expected specification and the implementation of it were identified and were relayed to the team, however they pose no type of vulnerability and concern an optional code path that was unaccounted for. The project has adequate doumentation and specification outside of the source files, however the code comment coverage was minimal. --- ## <img src="https://svgshare.com/i/Pp1.svg" width="40"/> Recommendations Overall, the codebase of the contracts should be refactored to assimilate the findings of this report, enforce linters and / or coding styles as well as correct any spelling errors and mistakes that appear throughout the code to achieve a high standard of code quality and security. --- ## <img src="https://svgshare.com/i/Pp1.svg" width="40"/> Findings | ID | Title | Type | Severity |Resolved | | ---------: | ---------------------------------- | ----------------------- | ------------- |---- | | Exhibit-01 | Unlocked Compiler Version Declaration | Language Sepcific| Informational |<img src="https://s3.ax1x.com/2021/01/06/sEGTsI.png" style="zoom:4%;" /> | | Exhibit-02 | Incorrect Naming Convention Utilization | Coding Style| Informational |<img src="https://s3.ax1x.com/2021/01/06/sEYm4S.png" style="zoom:3.8%;" /> | | Exhibit-03 | Incorrect Order of Layout Utilization | Coding Style| Informational |<img src="https://s3.ax1x.com/2021/01/06/sEGTsI.png" style="zoom:4%;" /> | | Exhibit-04 | Proper Usage of “public” and “external” type | Gas Optimization|Informational|<img src="https://s3.ax1x.com/2021/01/06/sEGTsI.png" style="zoom:4%;" /> | | Exhibit-05 | Lack of natspec comments| Optimization | Informational |<img src="https://s3.ax1x.com/2021/01/06/sEYm4S.png" style="zoom:3.8%;" /> | | Exhibit-06 | Use SafeMath | Mathematical Operations | Informational |<img src="https://s3.ax1x.com/2021/01/06/sEYm4S.png" style="zoom:3.8%;" /> | | Exhibit-07 | State variables that could be declared constant| Gas Optimization | Informational|<img src="https://s3.ax1x.com/2021/01/06/sEGTsI.png" style="zoom:4%;" /> | | Exhibit-08 | Missing Emit Events| Optimization| Minor |<img src="https://s3.ax1x.com/2021/01/06/sEYm4S.png" style="zoom:3.8%;" /> | | Exhibit-09 | Events Should Add Indexed Keyword| Optimization| Informational |<img src="https://s3.ax1x.com/2021/01/06/sEYm4S.png" style="zoom:3.8%;" /> | | Exhibit-10 | Missing Checks| Logical Issue| Minor |<img src="https://s3.ax1x.com/2021/01/06/sEGTsI.png" style="zoom:4%;" /> | | Exhibit-11 | Code Redundancy| Optimization| Informational |<img src="https://s3.ax1x.com/2021/01/06/sEGTsI.png" style="zoom:4%;" /> | | Exhibit-12 | Potential Overflow| Mathematical Operations| Minor | <img src="https://s3.ax1x.com/2021/01/06/sEGTsI.png" style="zoom:4%;" /> | --- ### <a name="UNP-01" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-01: Unlocked Compiler Version Declaration | Type | Severity | Location | | ----------------- | ------------- | ------------------------------------------------------------ | | Language Sepcific | Informational | [SLNToken.sol, StarPools.sol, WordFund.sol, DebugLogs.sol](#) | #### Description: The compiler version utilized throughout the project uses the “^” prefix specifier, denoting that a compiler version which is greater than the version will be used to compile the contracts. Recommend the compiler version should be consistent throughout the codebase. #### Recommendation: It is a general practice to instead lock the compiler at a specific version rather than allow a range of compiler versions to be utilized to avoid compiler-specific bugs and be able to identify ones more easily. We recommend locking the compiler at the lowest possible version that supports all the capabilities wished by the codebase. This will ensure that the project utilizes a compiler version that has been in use for the longest time and as such is less likely to contain yet-undiscovered bugs. #### Alleviation: The team heeded our advice and locked the version of their contracts at version 0.6.12, ensuring that compiler-related bugs can easily be narrowed down should they occur. The recommendations were applied in commit ac56f1dfb4fe37c7af01cbd50fd6b0cf9e0edf2a. ### <a name="UNP-02" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-02: Incorrect Naming Convention Utilization | Type | Severity | Location | |-|-|-| | Coding Style | Informational | [SLNToken.sol, StarPools.sol, WordFund.sol ](#) | #### Description: Solidity defines a naming convention that should be followed. In general, parameters should use mixedCase, refer to: https://solidity.readthedocs.io/en/v0.6.0/style-guide.html#naming-conventions Events should be named using the CapWords style. Examples: Events like: `addWord`, `biddingWord`, `harvestWord`, `releaseWord`, `setWordData`, `claimInvoked`, `factorReset`, `depositInvoked`, `withdrawInvoked`, `depositMoveInvoked`, `joinInvoked`, `quitInvoked`, `claimInvoked`, `claimMoveInvoked`, `claimValue` Parameter shoud use mixedCase. Examples: Parameter like:`_poolid`,`_topoolid`,`_toaccount`, `_wordid`, `_wordlist` Structs should be named using the CapWords style. Examples: Struct like:`worddata` #### Recommendation: The recommendations outlined here are intended to improve the readability, and thus they are not rules, but rather guidelines to try and help convey the most information through the names of things. #### Alleviation: No alleviation. ### <a name="UNP-03" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-03: Incorrect Order of Layout Utilization | Type | Severity | Location | |-|-|-| | Coding Style | Informational | [SLNToken.sol, StarPools.sol, WordFund.sol ](#) | #### Description: Solidity defines an Order of Layout that should be followed. In general, inside each contract, library or interface, use the following order: 1. Type declarations 2. State variables 3. Events 4. Functions refer to: https://docs.soliditylang.org/en/v0.6.0/style-guide.html?highlight=layout#order-of-layout #### Recommendation: See Exhibit 2. #### Alleviation: The team heeded our advice and fixed the order of layout. The recommendations were applied in commit ac56f1dfb4fe37c7af01cbd50fd6b0cf9e0edf2a. ### <a name="UNP-04" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-04: Proper Usage of "public" and "external" type | Type | Severity | Location | | ------------ | ------------- | ----------------------------------------------------------- | | Gas Optimization | Informational | [SLNToken.sol, StarPools.sol, WordFund.sol](#) | #### Description: "public" functions that are never called by the contract could be declared "external" . When the inputs are arrays "external" functions are more efficient than "public" functions. Examples: Functions `setTeamAddress()`,`setPoolAddress()`, `tokensThisWeek()`, `claim()`, `setBlockedlist()` in contract `SLNToken`. Functions `poolLength()`, `addPool()`, `setPool()`, `setLiveFactor()`, `deposit()`, `withdraw()`, `depositMove()`, `claim()`, `claimMove()`, `annualPerShare()` in contract `StarPools`. Functions `setRelatedPoolId()`, `wordsLength()`, `addWords()`, `setData()`, `bidding()`, `harvest()`, `release()`, `claim()` in contract `WordFund`. #### Recommendation: Consider using the "external" attribute for functions never called from the contract. #### Alleviation: The team heeded our advice and used the "external" attribute for functions never called from the contract. The recommendations were applied in commit ac56f1dfb4fe37c7af01cbd50fd6b0cf9e0edf2a. ### <a name="UNP-05" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-05: Lack of natspec comments | Type | Severity | Location | | --------- | ------------- | ---------------------------------------------------- | | Optimization | Informational | [SLNToken.sol, StarPools.sol, WordFund.sol](#) | #### Description: Contract code is missing natspec comments, which helps understand the code and all the functions' parameters. #### Recommendation: Please follow these style guides for adding natspec comments. https://docs.soliditylang.org/en/v0.6.11/style-guide.html?highlight=natspec%23natspec #### Alleviation: No alleviation. ### <a name="UNP-06" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-06: Use SafeMath | Type | Severity | Location | | ----------------------- | ------------- | -------------------------------- | | Mathematical Operations | Informational | [SLNToken.sol, StarPools.sol, WordFund.sol](#) | #### Description: Many functions in the `grant` contract did not use SafeMath. Example: Functions `_makeSum()`,`_makeLastWeek() `, `tokensThisWeek()`,`_calcMintValue()`,`calcPoolValue()`,`teamClaim()` in contract `SLNToken`. Functions `addPool()`,`_profitnow()`,`_quit()`,`_join()`,`balanceOf() `,`annualPerShare()` in contract `StarPools`. Functions `lowAmount()`,`bidding()`,`harvest()`,`_ishold()`,`release() ` in contract `WordFund`. #### Recommendation: We recommend to use SafeMath for calculations. #### Alleviation: No alleviation. ### <a name="UNP-07" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-07: State variables that could be declared constant | Type | Severity | Location | |-|-|-| | Gas Optimization | Informational | [WordFund.sol L43](#) | #### Description: Constant state variables should be declared constant to save gas. ```Solidity uint256 public biddingLockingPeriod = (4 hours); uint256 public positionLockingPeriod = (15 days); ``` #### Recommendation: Consider to add the constant attributes to state variables that never change. ```Solidity uint256 public constant biddingLockingPeriod = (4 hours); uint256 public constant positionLockingPeriod = (15 days); ``` #### Alleviation: The team heeded our advice and defined the un-changed variables as constants. The recommendations were applied in commit ac56f1dfb4fe37c7af01cbd50fd6b0cf9e0edf2a. ### <a name="UNP-08" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-08: Missing Emit Events | Type | Severity | Location | | ------------ | -------- | ------------------------------------------------------------ | | Optimization | Minor | [SLNToken.sol L44,L48, StarPools.sol L42,L51,L60, WordFund.sol L21, L25](#) | #### Description: Several sensitive actions are defined without event declarations. Examples: Functions like : `setPoolAddress()` , `setRelatedPoolId` , `setTeamAddress()`, `addPool()`, `setPool()`, `setLiveFactor()` #### Recommendation: Consider adding events for sensitive actions, and emit it in the function like below. ``` event SetPoolAddress(address indexed poolAddress); function setPoolAddress(address _poolAddress) public onlyOwner { poolAddress = _poolAddress; _approve(address(this), poolAddress, totalSupply()*835/1000); emit SetPoolAddress(poolAddress); } ``` #### Recommendation: Change the condition to check inequality with zero, as it is more efficient regarding unsigned integer variables. #### Alleviation: No alleviation. ### <a name="UNP-09" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-09: Events Should Add Indexed Keyword | Type | Severity | Location | |-|-|-| | Optimization | Informational | [SLNToken.sol L37, StarPools.sol L82,L164,L296 WordFund.sol L53](#) | #### Description: Event definittions in contract `SLNToken`,`StarPools` and `WordFund` do not have `indexed` keyword. The indexed parameters for logged events will allow you to search for these events using the indexed parameters as filters. Examples: ```Solidity event claimInvoked(address from, uint256 poolid, uint256 wordid, address to); ``` #### Recommendation: We recommend to add the `indexed` keyword. ```Solidity event claimInvoked(address indexed from, uint256 indexed poolid, uint256 indexed wordid, address indexed to); ``` #### Alleviation: No alleviation. ### <a name="UNP-10" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-10: Missing Checks | Type | Severity | Location | | ----------------------- | ------------- | -------------------------------- | | Logical Issue | Minor | [SLNToken.sol L17,L44,L129 StarPools.sol L14,L42,L51 WordFund.sol L21,L108](#) | #### Description: Function `setTeamAddress`, `setPoolAddress`, `constructor` in contract SLNToken, `constructor` in contract StarPools, `setPoolAddress` in contract WordFund are missing zero address checks. Function `setBlockedlist()` in contract SLNToken, `setPool()` in contract StarPools, `bidding()` in contract WordFund in are missing parameter value checks. Function `addPool()` in contract StarPools is missing parameter checks for `_LPToken`, same `_LPToken` can be used multiple times to add multiple pools. #### Recommendation: We recommend to add neccessary checks, for example: ``` function setTeamAddress(address _teamAddress) public onlyOwner { require(_teamAddress != address(0), "zero address is not allowed!"); teamAddress = _teamAddress; } function setBlockedlist(address _address, bool _blocked) public onlyOwner { require(blockedlist[_address] != _blocked); blockedlist[_address] = _blocked; } function setPool(uint256 _poolid, address _LPLimit, uint256 _start, uint256 _ending, bool _paused) public onlyOwner returns (uint256) { require(_poolid < pools.length , "pool id is not existing!"); pools[_poolid].LPLimit = _LPLimit; ... } function bidding(uint256 _wordid, uint256 _poolid, uint256 _value, address _referrer) public returns (uint256) { ... address LPAddress = getPoolLPToken(poolId); require(LPAddress != address(0) , "pool id is not existing!"); ... } ... ``` #### Alleviation: The team heeded our advice and added the checks. The recommendations were applied in commit ac56f1dfb4fe37c7af01cbd50fd6b0cf9e0edf2a. ### <a name="UNP-11" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-11: Code Redundancy | Type | Severity | Location | | ----------------------- | ------------- | -------------------------------- | | Optimization | Informational | [StarPools.sol L298](#) | #### Description: Return value for function `_setInviter()` is never used. #### Recommendation: Consider to remove un-used return value to reduce code redundancy. #### Alleviation: The team heeded our advice and removed the un-used return value. The recommendations were applied in commit ac56f1dfb4fe37c7af01cbd50fd6b0cf9e0edf2a. ### <a name="UNP-12" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-12: Potential Overflow | Type | Severity | Location | | ----------------------- | ------------- | -------------------------------- | | Mathematical Operations | Minor | [WordFund.sol L62](#) | #### Description: Variable `weight` in function `lowAmount()` is not checked for zero value. This can lead to an overflow, due to the nature of the divide operation. #### Recommendation: Consider to add a check for the variable `weight` before devide operation. ``` function lowAmount() public view returns (uint256) { uint256 weight; (,,,,weight,,,) = StarPools(poolAddress).pools(poolId); require (weight != 0, "weight is zero") return 100*(10**18)*(10**9)/weight; } ``` #### Alleviation: The team heeded our advice and added the check. The recommendations were applied in commit ac56f1dfb4fe37c7af01cbd50fd6b0cf9e0edf2a. <div STYLE="page-break-after: always;"></div> ### **Appendix** ------ ### **Finding Categories** **Gas Optimization** Gas Optimization findings refer to exhibits that do not affect the functionality of the code but generate different, more optimal EVM opcodes resulting in a reduction on the total gas cost of a transaction. **Mathematical Operations** Mathematical Operation exhibits entail findings that relate to mishandling of math formulas, such as overflows, incorrect operations etc. **Logical Issue** Logical Issue findings are exhibits that detail a fault in the logic of the linked code, such as an incorrect notion on how `block.timestamp` works. **Control Flow** Control Flow findings concern the access control imposed on functions, such as owner-only functions being invoke-able by anyone under certain circumstances. **Volatile Code** Volatile Code findings refer to segments of code that behave unexpectedly on certain edge cases that may result in a vulnerability. **Data Flow** Data Flow findings describe faults in the way data is handled at rest and in memory, such as the result of a `struct` assignment operation affecting an in-memory `struct` rather than an instorage one. **Language Specific** Language Specific findings are issues that would only arise within Solidity, i.e. incorrect usage of `private` or `delete` . **Coding Style** Coding Style findings usually do not affect the generated byte-code and comment on how to make the codebase more legible and as a result easily maintainable. **Inconsistency** Inconsistency findings refer to functions that should seemingly behave similarly yet contain different code, such as a `constructor` assignment imposing different `require` statements on the input variables than a setter function. **Magic Numbers** Magic Number findings refer to numeric literals that are expressed in the codebase in their raw format and should otherwise be specified as `constant` contract variables aiding in their legibility and maintainability. **Compiler Error** Compiler Error findings refer to an error in the structure of the code that renders it impossible to compile using the specified version of the project. **Dead Code** Code that otherwise does not affect the functionality of the codebase and can be safely omitted. ------ ### **Icons explanation** &nbsp;<img src="https://s3.ax1x.com/2021/01/06/sEGTsI.png" style="zoom:4%;" /> : Issue resolved &nbsp;<img src="https://s3.ax1x.com/2021/01/06/sEYm4S.png" style="zoom:3.8%;" /> : Issue not resolved / Acknowledged. The team will be fixing the issues in the own timeframe. <img src="https://s3.ax1x.com/2021/01/06/sE8IbV.png" style="zoom:5.2%;" /> : Issue partially resolved. Not all instances of an issue was resolved.