Gainswap CertiK Preliminary Comments For Gainswap

--- title: Gainswap CertiK Preliminary Comments For Gainswap tags: pre-report --- {%hackmd XdIXzOf5Ty2M3Uj1taFHIg %} <center> <img src="https://www.wing.vc/uploads/images/companies/certik-logo04.png" height="232" /> </center> Preliminary Comments Security Assessment December 8th, 2020 Preliminary Report For : Gainswap team @ Gainswap 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">Project Name</td> <td width="50%" valign="top"><a href="https://github.com/gainswap/gainswap">Gainswap Protocol</a></td> </tr> <tr> <td width="50%" valign="top">Description</td> <td width="50%" valign="top">a defi platform with swap and staking functionalities</td> </tr> <tr> <td width="50%" valign="top">Platform</td> <td width="50%" valign="top">Ethereum; Solidity</td> </tr> <tr> <td width="50%" valign="top">Codebase</td> <td width="50%" valign="top"><a href="https://github.com/gainswap/gainswap">GitHub Repository</a></td> </tr> <tr> <td width="50%" valign="top">Commit</td> <td width="50%" valign="top"> <a href="https://github.com/gainswap/gainswap/commit/866ccbe7e1ed9007f5e32c784288537a01d1d29f"> 866ccbe7e1ed9007f5e32c784288537a01d1d29f</a> </td> </tr> </table> #### Audit Summary <table> <tr> <td width="50%" valign="top">Delivery Date</td> <td width="50%" valign="top">Dec. 8, 2020</td> </tr> <tr> <td width="50%" valign="top">Method of Audit</td> <td width="50%" valign="top">Static Analysis, Manual Review</td> </tr> <tr> <td width="50%" valign="top">Consultants Engaged</td> <td width="50%" valign="top">2</td> </tr> <tr> <td width="50%" valign="top">Timeline</td> <td width="50%" valign="top">Dec. 4, 2020 - Dec. 8, 2020</td> </tr> </table> --- ## <img src="https://svgshare.com/i/Pp1.svg" width="40"/> Executive Summary This report has been prepared for **Gainswap** protocol 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. --- | ID | Title | Type | Severity | | -----: | -------------------------------------------- | ------------ | ------------- | | EXH-01 | Incorrect File Name | Optimization | Informational | | EXH-02 | Compilation Issues | Compilation | Major | | EXH-03 | Incorrect Naming Convention Utilization | Coding Style | Informational | | EXH-04 | Proper Usage of "public" and "external" type | Optimization | Informational | | EXH-05 | Controversial specifications in whitepaper | Optimization | Informational | | EXH-06 | Security risk of transferring assets | Security | Major | | EXH-07 | Incorrect logic for `_withdraw0` | Optimization | Informational | | EXH-08 | Math Overflow | Optimization | Minor | | EXH-09 | Missing Emit Events | Optimization | Minor | | EXH-10 | Gas Consumption | Optimization | Informational | | EXH-11 | Check Zero Address | Optimization | Informational | --- ### <a name="UNP-01" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-01: Incorrect File Name | Type | Severity | Location | |-|-|-| | Optimization | Major | [GainswapFactory.sol,GainswapRouter02.sol](#) | #### Description: There are no extension file names for file <a href="https://github.com/gainswap/gainswap/blob/main/contract/GainswapFactory"> GainswapFactory</a> and file <a href="https://github.com/gainswap/gainswap/blob/main/contract/GainswapRouter02"> GainswapRouter02</a>. #### Recommendation: We recommend to add the extension file name as below: **GainswapFactory.sol** **GainswapRouter02.sol** ### <a name="UNP-02" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-02: Compilation Warning | Type | Severity | Location | |-|-|-| | Compilation | Major | [GainswapFactory.sol,GainswapRouter02.sol](#) | #### Description: Contract code size exceeds 24576 bytes (a limit introduced in Spurious Dragon). This contract may not be deployable on mainnet. Consider enabling the optimizer (with a low "runs" value!), turning off revert strings, or using libraries. Contract creation initialization returns data with length of more than 24576 bytes. The deployment will likely fails. #### Recommendation: We recommend to split the GainswapFactory contract and GainswapPair contract into different files. ### <a name="UNP-03" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-03: Discussion | Type | Severity | Location | |-|-|-| | Discussion | Informational | [GainswapFactory.sol](#) | #### Description: We are not sure about the concept of virtual liquidity? Does dummy0 represent the virtual liquidity? Looks like the dummy0 will impact the amount of GainswapERC20 tokens to be minted? ```Solidity function mint(address to) external lock returns (uint liquidity) { (uint112 _reserve0, uint112 _reserve1,) = getReserves(); // gas savings uint balance0 = b0(); uint balance1 = b1(); uint amount0 = balance0.sub(_reserve0); uint amount1 = balance1.sub(_reserve1); _reserve0 -= dummy0; _reserve1 -= dummy1; uint _totalSupply = totalSupply; // gas savings if (_totalSupply == 0) { liquidity = Math.sqrt(amount0.mul(amount1)).sub(MINIMUM_LIQUIDITY); _mint(address(0), MINIMUM_LIQUIDITY); // permanently lock the first MINIMUM_LIQUIDITY tokens } else { liquidity = Math.min(amount0.mul(_totalSupply) / _reserve0, amount1.mul(_totalSupply) / _reserve1); } ...... _mint(to, liquidity); ...... } ``` ### <a name="UNP-04" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-04: Discussion | Type | Severity | Location | |-|-|-| | Discussion | Informational | [GainswapFactory.sol L501](#) | #### Description: According to the chapter 2.3 in doc <a href="https://github.com/gainswap/gainswap/blob/main/gainswap.pdf"> gainswap.pdf</a>, the fee is 0.30%. `This is effectively the same as letting anyone flash-borrow any of assets stored in a Gainswap pool (for the same 0.30% fee as Uniswap charges for trading).` But in chapter 3.3 in the doc : `The 30-base-point fee fixed on Uniswap can be negotiated and formulated by the community in Gainswap.That can be changed according to user needs when new situations appear.` ```Solidity function setFee(uint8 _fee) external onlyOwner() { fee = _fee; emit FeeUpdated(_fee); } ``` ### <a name="UNP-05" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> Exhibit-05: Discussion | Type | Severity | Location | |-|-|-| | Discussion | Minor | [GainswapFactory.sol L528](#) | #### Description: This protocol has an external dependency. User's digital currencies can be deposited to a third-party service (like YFI). The system should only be used if the service is appropriately trusted. The governace privilege should be controlled. `yToken0` and `yToken1` can be easily set by governace. This will change the address where user's digital currencies to be deposited. ```Solidity function setY0(address y) public onlyOwner() { yToken0 = y; emit Y0Updated(y); approve0(); } ``` #### Recommendation: We recommend to move the governace to Timelock or community after the protocol deployed.