--- title: YFBTC CertiK Preliminary Comments For YFBTC Protocol tags: pre-report --- <div><div style="background-image:url(https://i.imgur.com/n9A7QyA.png); height: 800px;background-position: center; background-repeat: no-repeat;background-size: cover;"><div style="position: relative;top: 25%;left:5.6%;line-height: 2.5em; color: white;"> <p style="font-size: 28px">Preliminary Comments</p> <p style="font-size: 22px">Security Assessment</p> <p style="font-size: 18px">January 27th, 2021</p> <p style="font-size: 18px; color: darkred">Preliminary Report</p> </div> <div style="position: relative;top: 44%;left:5.6%; color: #8D8D8D;">For : <p style="font-size: 12px; color: #C6CCD3">YFBTC Protocol</p> <p style="font-size: 11px;color: #C6CCD3; line-height: 1.5rem;">By :</p> <p style="font-size: 12px; line-height: 0.5rem; color: #C6CCD3;">Owan Li @ CertiK</p> <p style="font-size: 12px; line-height: 0.055rem; color:#F6AA42">guilong.li@certik.org</p> <br/> <p style="font-size: 11px; line-height: 0.5rem; color: #C6CCD3;">Bryan Xu @ CertiK</p> <p style="font-size: 12px; line-height: 0.03rem; color:#F6AA42">buyun.xu@certik.org</p></div></div></div> <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> ## <img src="https://svgshare.com/i/Pp1.svg" width="40"/> Disclaimer CertiK reports are not, nor should be considered, an “endorsement” or “disapproval” of any particular project or team. These reports are 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 CertiK to perform a security review. CertiK Reports do 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. CertiK Reports should not be used in any way to make decisions around investment or involvement with any particular project. These reports in no way provide investment advice, nor should be leveraged as investment advice of any sort. CertiK Reports represent an extensive auditing 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. CertiK’s position is that each company and individual are responsible for their own due diligence and continuous security. CertiK’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. ### What is a CertiK report? * A document describing in detail an in depth analysis of a particular piece(s) of source code provided to CertiK by a Client. * An organized collection of testing results, analysis and inferences made about the structure, implementation and overall best practices of a particular piece of source code. * Representation that a Client of CertiK has indeed completed a round of auditing with the intention to increase the quality of the company/product’s IT infrastructure and or source code. <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> ## <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/Yfswapfinance/YFBTC-Contract">YFBTC</a></td> </tr> <tr> <td width="50%" valign="top"><b>Description</b></td> <td width="50%" valign="top">an AMM based Dex with DeFi farming, staking and a Dex Exchange functionalities</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/Yfswapfinance/YFBTC-Contract">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/Yfswapfinance/YFBTC-Contract/commit/3e0cea0ed8d639d2a69e2245f1dbc1d5e11886e5"> 3e0cea0ed8d639d2a69e2245f1dbc1d5e11886e5</a><br/> <a href="https://github.com/Yfswapfinance/YFBTC-Contract/commit/84d17f5d0b7faeb7ccc6f71d44f80d9a49b9a661">84d17f5d0b7faeb7ccc6f71d44f80d9a49b9a661</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. 27th, 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. 20 - Jan. 22, 2021 & Jan 27, 2021</td> </tr> </table> #### Vulnerability Summary <table> <tr> <td width="50%" valign="top"><b>Total Issues</b></td> <td width="50%" valign="top">18</td> </tr> <tr> <td width="50%" valign="top"><b>Total Critical</b></td> <td width="50%" valign="top">3</td> </tr> <tr> <td width="50%" valign="top"><b>Total Major</b></td> <td width="50%" valign="top">3</td> </tr> <tr> <td width="50%" valign="top"><b>Total Minor</b></td> <td width="50%" valign="top">2</td> </tr> <tr> <td width="50%" valign="top"><b>Total Informational</b></td> <td width="50%" valign="top">10</td> </tr> </table> <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> ## <img src="https://svgshare.com/i/Pp1.svg" width="40"/> Executive Summary This report has been prepared for **YFBTC** 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. <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> --- ## <img src="https://svgshare.com/i/Pp1.svg" width="40"/> File in Scope |ID| Contract | SHA-256 Checksum | | ----------------- |----------------- | ------------------------------------------------------------ | | **YFB** | **YFBTC.sol** | 8d528710e75dc5e6eb93e77d663633aa7f200b0f0ca2febe857543728113ec08 | | **MCF** | **MasterChef.sol** | 693402da87aab21d2175181d26c7cfd249ab58bc8e847ca44c298021e710a3b2 | | **UVP** | **UniswapV2Pair.sol** | c70b2cda77719b06922433870231857518bde8efba9730a677cc7a0b1336f923 | | **USF** | **IUniSwapV2Factory.sol** | 01fc78d2e725b9885e4f16337f937529553fc2660c1e46c4b408db78cb1d6c78 | --- | ID | Title | Type | Severity | Resolved | | ---------: | ----------------------------------------------- | ----------------------- | ------------- | ------------------------------------------------------------ | | MCF-01 | Compilation Error | Compilation | Major | <img src="https://s3.ax1x.com/2021/01/06/sEGTsI.png" style="zoom:3%;" /> | | MCF-02 | Incorrect Naming Convention Utilization | Coding Style | Informational | <img src="https://s3.ax1x.com/2021/01/06/sEGTsI.png" style="zoom:3%;" /> | | MCF-03 | Deprecated Syntax | Language Specific | Informational | <img src="https://s3.ax1x.com/2021/01/06/sEGTsI.png" style="zoom:3%;" /> | | MCF-04 | Local variable shadowing | Coding Style | Informational | <img src="https://s3.ax1x.com/2021/01/06/sEYm4S.png" style="zoom:3.8%;" /> | | MCF-05 | Too Many Digits | Coding Style | Informational | <img src="https://s3.ax1x.com/2021/01/06/sEYm4S.png" style="zoom:3.8%;" /> | | MCF-06 | State variables that could be declared constant/immutable | Gas Optimization | Informational | <img src="https://s3.ax1x.com/2021/01/06/sEYm4S.png" style="zoom:3.8%;" /> | | MCF-07 | Proper Usage of “public” and “external” type | Gas Optimization | Informational | <img src="https://s3.ax1x.com/2021/01/06/sEGTsI.png" style="zoom:3%;" /> | | MCF-08 | Missing Emit Events | Optimization | Minor | <img src="https://s3.ax1x.com/2021/01/06/sEGTsI.png" style="zoom:3%;" /> | | MCF-09 | Missing Some Important Checks | Logical Issue| Minor | <img src="https://s3.ax1x.com/2021/01/06/sEGTsI.png" style="zoom:3%;" /> | | MCF-10 | Transfer Balance less than `_amount` | Logical Issue| Major | <img src="https://s3.ax1x.com/2021/01/06/sEGTsI.png" style="zoom:3%;" /> | | MCF-11 | Code Redundancy | Dead Code| Informational | <img src="https://s3.ax1x.com/2021/01/06/sEYm4S.png" style="zoom:3.8%;" /> | | MCF-12 | Duplicate Codes | Optimization| Informational | <img src="https://s3.ax1x.com/2021/01/06/sEYm4S.png" style="zoom:3.8%;" /> | | MCF-13 | Contracts Calling Arbitrary Functions | Optimization | Critical | <img src="https://s3.ax1x.com/2021/01/06/sEYm4S.png" style="zoom:3.8%;" /> | | MCF-14 | Potential Underflow | Mathematical Operations | Informational | <img src="https://s3.ax1x.com/2021/01/06/sEYm4S.png" style="zoom:3.8%;" /> | | MCF-15 | Proper Usage for `storage` and `memory` | Gas Optimization | Informational | <img src="https://s3.ax1x.com/2021/01/06/sEYm4S.png" style="zoom:3.8%;" /> | | MCF-16 | Logical Issue in Function `update()` | Logical Issue | Major | <img src="https://s3.ax1x.com/2021/01/06/sEGTsI.png" style="zoom:3%;" /> | |MCF-17| Incorrect Require Conditions| Logical Issue | Critical | <img src="https://s3.ax1x.com/2021/01/06/sEYm4S.png" style="zoom:3.8%;" /> | | YFB-01 | Transfer without `_moveDelegates` | Optimization | Critical | <img src="https://s3.ax1x.com/2021/01/06/sEYm4S.png" style="zoom:3.8%;" /> | <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> ### <a name="UNP-01" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> MCF-01: Compilation Error | Type | Severity | Location | |-|-|-| | Compilation | Major | [MasterChef.sol L6,L11 YFBTC.sol L4](#) | #### Description: 1. Identifier already declared error. The previous declaration is here : MasterChef.sol L6: `import @openzeppelin/contracts/token/ERC20/IERC20.sol;` Then it is declared again here: YFBTC.sol L5: `import "IERC20.sol";` 2. Source "YFBitcoin.sol" not found error: File not found. `import "./YFBitcoin.sol";` #### Recommendation: 1. We recommend to remove these files from this repo: `AddressLib.sol`, `context.sol`, `ERC20.sol`, `IERC20.sol`, `Ownable.sol`, `SafeMath.sol`. And then import the files from @openzeppelin in YFBTC.sol. 2. We recommend to change to below import: `import "./YFBTC.sol";` #### Alleviation: The team heeded our advice and resolved this issue in commit <a href="https://github.com/Yfswapfinance/YFBTC-Contract/commit/1ebeeb3cdc40523af229eeb9ef9a569bcc58aee2#diff-b094db7ce2f99cbcbde7ec178a6754bac666e2192f076807acbd70d49ddd0559">1ebeeb3cdc40523af229eeb9ef9a569bcc58aee2</a>. <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> ### <a name="UNP-02" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> MCF-02: Incorrect Naming Convention Utilization | Type | Severity | Location | | ------------ | ------------- | ---------------------------------- | | Coding Style | Informational | [MasterChef.sol L53](#) | #### 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.12/style-guide.html#naming-conventions Constants shoud use UPPER_CASE_WITH_UNDERSCORES. Examples: Constants like: `yfbtcMultiplier` #### 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: The team heeded our advice and renamed the constant with UPPER_CASE_WITH_UNDERSCORES. The recommendations were applied in commit <a href="https://github.com/Yfswapfinance/YFBTC-Contract/commit/1ebeeb3cdc40523af229eeb9ef9a569bcc58aee2#diff-b094db7ce2f99cbcbde7ec178a6754bac666e2192f076807acbd70d49ddd0559">1ebeeb3cdc40523af229eeb9ef9a569bcc58aee2</a>. <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> ### <a name="UNP-03" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> MCF-03: Deprecated Syntax | Type | Severity | Location | | ---------------- | ------------- | -------------------------- | | Language Specific | Informational | [MasterChef.sol L394](#) | #### Description: Using ".value(...)" is deprecated. Use "{value: ...}" instead. ```Solidity (bool success, bytes memory result) = _target.call.value(_value)(_data); ``` #### Recommendation: We recommend to change the codes like below: ```Solidity (bool success, bytes memory result) = _target.call{ value: _value }(_data); ``` #### Alleviation: The team heeded our advice and resolved this issue in commit <a href="https://github.com/Yfswapfinance/YFBTC-Contract/commit/1ebeeb3cdc40523af229eeb9ef9a569bcc58aee2#diff-b094db7ce2f99cbcbde7ec178a6754bac666e2192f076807acbd70d49ddd0559">1ebeeb3cdc40523af229eeb9ef9a569bcc58aee2</a>. <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> ### <a name="UNP-04" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> MCF-04: Local variable shadowing | Type | Severity | Location | | ------------ | ------------- | ---------------------------------- | | Coding Style | Informational | [MasterChef.sol L174 ](#) | #### Description: Detection of shadowing using local variables. Variable `rewardPerBlock` in `getMultiplier()` function shadows `rewardPerBlock()` function. #### Recommendation: We recommend to rename the local variables that shadow another component. #### Alleviation: No alleviation. <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> ### <a name="UNP-05" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> MCF-05: Too Many Digits | Type | Severity | Location | | ------------ | ------------- | -------------------------------- | | Coding Style | Informational | [MasterChef.sol L174](#) | #### Description: Literals with many digits are difficult to read and review. Examples: ```Solidity uint256 rewardPerBlock = 26871140040000000; ``` #### Recommendation: Consider to use Ether suffix or the scientific notation. #### Alleviation: No alleviation. <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> ### <a name="UNP-06" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> MCF-06: State variables that could be declared constant/immutable | Type | Severity | Location | | ---------------- | ------------- | -------------------------- | | Gas Optimization | Informational | [MasterChef.sol L71,L77,L79 YFBTC.sol L21](#) | #### Description: Constant state variables should be declared constant to save gas. ```Solidity uint256 public bonusEndBlock; uint256 public totalAllocPoint = 0; uint256 public devFee = 300; ``` Below variables change only once, better to define them as immutable to avoid gas consumption. ```Solidity uint256 public startBlock; address public token0; address public token1; address public factory; YFBitcoin public yfbtc; uint32 public blockTimestampLast; uint256 public cap; ``` #### Recommendation: Add constant attributes to state variables that never change. Add immutable attributes to state variables that only change once. We recommend to change the codes like below examples: ```Solidity uint256 public constant devFee = 300; uint256 public immutable startBlock; ``` #### Alleviation: No Alleviation. <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> ### <a name="UNP-07" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> MCF-07: Proper Usage of "public" and "external" type | Type | Severity | Location | | ---------------- | ------------- | ------------------------------------------------ | | Gas Optimization | Informational | [MasterChef.sol YFBTC.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 `transfer()`, `transferFrom()`,`setDevAddress()`, `setTransferFee()`, `mint()` in contract `YFBitcoin`. Functions `deposit()`, `setDevAddress()`, `setTransferFee()`, `add()`,`deposit()`,`withdraw()` in contract `YFBTCMaster`. #### Recommendation: Consider using the "external" attribute for functions never called from the contract. #### Alleviation: The team heeded our advice and resolved this issue in commit <a href="https://github.com/Yfswapfinance/YFBTC-Contract/commit/1ebeeb3cdc40523af229eeb9ef9a569bcc58aee2#diff-b094db7ce2f99cbcbde7ec178a6754bac666e2192f076807acbd70d49ddd0559">1ebeeb3cdc40523af229eeb9ef9a569bcc58aee2</a>. <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> ### <a name="UNP-08" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> MCF-08: Missing Emit Events | Type | Severity | Location | | ------------ | -------- | -------------------------- | | Optimization | Minor | [MasterChef.sol L117,L121 YFBTC.sol L38,L44](#) | #### Description: Several sensitive actions are defined without event declarations. Examples: Functions like : `constructor()`, `setDevAddress()` , `setTransferFee()` in both `YFBTCMaster` and `YFBitcoin` contract; and `updatePool()` in `YFBTCMaster`. #### Recommendation: Consider adding events for sensitive actions, and emit it in the function like below. ``` event SetDevAddress(address indexed _devAddress); function setDevAddress(address _devAddress) public onlyOwner { yfbtc.setDevAddress(_devAddress); emit SetDevAddress(_devAddress); } ``` #### Alleviation: The team heeded our advice and added events for sensitive actions. The recommendations were applied in commit <a href="https://github.com/Yfswapfinance/YFBTC-Contract/commit/1ebeeb3cdc40523af229eeb9ef9a569bcc58aee2#diff-b094db7ce2f99cbcbde7ec178a6754bac666e2192f076807acbd70d49ddd0559">1ebeeb3cdc40523af229eeb9ef9a569bcc58aee2</a>. <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> ### <a name="UNP-09" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> MCF-09: Missing Some Important Checks | Type | Severity | Location | | ------------- | -------- | -------------------------------------------- | | Logical Issue | Minor | [MasterChef.sol, YFBTC.sol ](#) | #### Description: Function `add()` in contract `YFBTCMaster` and function `setDevAddress()` in `YFBitcoin` are missing address zero checks for parameters. Functions `pendingReward()` ,`rewardPerBlock()`, `updatePool()`, `deposit()`, `withdraw()` and `emergencyWithdraw()` in contract `YFBTCMaster` are missing parameter `_pid` value checks. `constructor` in contract `YFBitcoin` should check whether parameter `_cap` greater than zero. #### Recommendation: We recommend to add neccessary checks, for example: ``` function add(IERC20 _lpToken) public onlyOwner { //EDIT commenting lastRewardBlock per pool (it is now common for all pools) //uint256 lastRewardBlock = block.number > startBlock ? block.number : startBlock; require (_lpToken != address(0), "MC: _lpToken should not be address zero"); for(uint i=0; i < poolInfo.length; i++){ require (poolInfo[i] != _lpToken, "MC: DO NOT add the same LP token more than once"); } poolInfo.push(PoolInfo({ lpToken: _lpToken, accYfbtcPerShare: 0, totalSupply: 0 })); } function pendingReward(uint256 _pid, address _user) external view returns (uint256) { PoolInfo storage pool = poolInfo[_pid]; require (pool != address(0), "MC: _pid is incorrect"); ... } ... ``` #### Alleviation: The team heeded our advice and resolved this issue in commit <a href="https://github.com/Yfswapfinance/YFBTC-Contract/commit/1ebeeb3cdc40523af229eeb9ef9a569bcc58aee2#diff-b094db7ce2f99cbcbde7ec178a6754bac666e2192f076807acbd70d49ddd0559">1ebeeb3cdc40523af229eeb9ef9a569bcc58aee2</a>. <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> ### <a name="UNP-10" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> MCF-10: Transfer balance less than _amount | Type | Severity | Location | | ------------- | -------- | -------------------------------------------- | | Logical Issue | Major | [MasterChef.sol L361 ](#) | #### Description: ``` // Safe yfbtcReward transfer function, just in case if rounding error causes pool to not have enough YFBTC. function safeYfbtcTransfer(address _to, uint256 _amount) internal { uint256 yfbtcBal = yfbtc.balanceOf(address(this)); if (_amount > yfbtcBal) { yfbtc.transfer(_to, yfbtcBal); } else { yfbtc.transfer(_to, _amount); } } ``` According to the `safeYfbtcTransfer()`, its intention is to transfer the `_amount` from `YFBTCMaster` contract to the desired `_to` address. If the `YFBTCMaster` contract has enough balance, it will transfer `_amount` to the destination. Else, it will just transfer the `yfbtcBal` to the destination. #### Recommendation: The advantage of 'safeYfbtcTransfer()' method in the protocol is that the administrator reserves the ability to transfer the main part of the amount to desitination under unexpected cases. It is also worthy of note the downside of 'safeYfbtcTransfer' method, where the destination address migth not receive the whole amount. Should emit events with parameters `_to` and `_amount` in case `_amount > yfbtcBal`. #### Alleviation: The team heeded our advice and resolved this issue in commit <a href="https://github.com/Yfswapfinance/YFBTC-Contract/commit/1ebeeb3cdc40523af229eeb9ef9a569bcc58aee2#diff-b094db7ce2f99cbcbde7ec178a6754bac666e2192f076807acbd70d49ddd0559">1ebeeb3cdc40523af229eeb9ef9a569bcc58aee2</a>. <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> ### <a name="UNP-11" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> MCF-11: Code Redundancy | Type | Severity | Location | | --------- | ------------- | ----------------------- | | Dead Code | Informational | [MasterChef.sol L321,L341,L355](#) | #### Description: Msg.sender is an address already. Use address keyword wrap up is meaningless and thus will spend gas. ```Solidity pool.lpToken.safeTransferFrom(address(msg.sender), address(this), _amount); ``` State variables are never used as below: ```Solidity uint256 public bonusEndBlock; uint256 public totalAllocPoint = 0; ``` #### Recommendation: We recommend to remove the address keyword like “address(msg.sender)”=>”msg.sender”; and remove the unused variables. #### Alleviation: No alleviation. <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> ### <a name="UNP-11" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> MCF-12: Duplicate Codes | Type | Severity | Location | | --------- | ------------- | ----------------------- | | Dead Code | Informational | [MasterChef.sol L238-L253](https://github.com/Yfswapfinance/YFBTC-Contract/blob/dfbf9558ca246e530016a0ccbe7c7ae12d08f51d/contracts/MasterChef.sol#L238),[L261-L277](https://github.com/Yfswapfinance/YFBTC-Contract/blob/dfbf9558ca246e530016a0ccbe7c7ae12d08f51d/contracts/MasterChef.sol#L261) | #### Description: The codes in functions `rewardPerBlock()` and `pendingReward()` have the same logic. #### Recommendation: We recommend to call the `rewardPerBlock()` function in `pendingReward()` to simplify the codes. #### Alleviation: No alleviation. <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> ### <a name="UNP-13" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> MCF-13: Internal Operations | Type | Severity | Location | | --------- | ------------- | ----------------------- | | Optimization | Major | [MasterChef.sol L420](#) | #### Description: <!-- Function `_invoke()` uses low-level calls. The use of low-level calls is error-prone. Low-level calls do not check for code existence or call success. When the interface of the target contract is not known at compile time, Solidity’s high-level syntax is unavailable. For these scenarios, the address type includes a `call()` function. Furthermore, the `_invoke()` function can be called by the administrator, it can call whatever functions others don't know.--> The function `_invoke()` adopts low-level call `_target.call`, which may cause errors when calling unknown external contracts/functions. In this specific low-level call, an arbitrary bytes `_data` is passed into it as one of argument. It has potential to allow `owner` to run any unknown function in the contract at the address of `_target`, which can also be set by `owner`. #### Recommendation: <!-- The use of low-level calls is error-prone. Low-level calls do not check for code existence or call success. Avoid low-level calls. Check the call success. If the call is meant for a contract, check for code existence--> Avoid using low-level calls with arbitrary arguments. If low-level call must be adopted, the existence of external contract must be checked. #### Alleviation: The development team has implemented this function to change some state for YFBTC contract like, they might needs to burn, update dev address, transfer fee or change ownership. (Certik -response) To update dev address, transfer fee or change ownership, functions `setDevAddress()`, `setTransferFee()` and `transferOwnership()` can be called. <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> ### <a name="UNP-14" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> MCF-14: Potential Underflow | Type | Severity | Location | | --------- | ------------- | ----------------------- | | Mathematical Operations | Informational | [MasterChef.sol L132](#) | #### Description: Below codes did not use SafeMath for subtraction. It is possible to lead to underflow. ```Solidity uint32 timeElapsed = blockTimestamp - blockTimestampLast; // overflow is desired ``` #### Recommendation: We recommend to use SafeMath for calculations. Consider to change the codes as below: ```Solidity uint32 timeElapsed = blockTimestamp.sub(blockTimestampLast,"update: subtraction underflow"); // overflow is desired ``` #### Alleviation: No alleviation. <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> ### <a name="UNP-15" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> MCF-15: Proper Usage for `storage` and `memory` | Type | Severity | Location | | --------- | ------------- | ----------------------- | | Gas Optimization | Informational | [MasterChef.sol L217](#) | #### Description: Function pendingReward() is an external view function. There is no meaning to define below variables as `storage`. ```Solidity PoolInfo storage pool = poolInfo[_pid]; UserInfo storage user = userInfo[_pid][_user]; ``` #### Recommendation: Consider changing `storage` to `memory`. #### Alleviation: No alleviation. <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> ### <a name="UNP-16" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> MCF-16: Logical Issue in Function `update()` | Type | Severity | Location | | --------- | ------------- | ----------------------- | | Logical Issue | Major | [MasterChef.sol L127](#) | #### Description: Variable `lastPrice` will only get value when `curretPrice < lastPrice`. There is no logics for case `curretPrice > lastPrice`. So `lastPrice` goes smaller and smaller. ```Solidity if ( curretPrice < lastPrice){ uint256 change = lastPrice.sub(curretPrice).mul(100).div(lastPrice); lastPrice = curretPrice; blockTimestampLast = blockTimestamp; if ( change >= 5 ) return false; } ``` #### Recommendation: Consider to add logic for `curretPrice > lastPrice` case. #### Alleviation: The team heeded our advice and resolved this issue in commit <a href="https://github.com/Yfswapfinance/YFBTC-Contract/commit/1ebeeb3cdc40523af229eeb9ef9a569bcc58aee2#diff-b094db7ce2f99cbcbde7ec178a6754bac666e2192f076807acbd70d49ddd0559">1ebeeb3cdc40523af229eeb9ef9a569bcc58aee2</a>. <div style="page-break-after: always; visibility: hidden"> \pagebreak </div> ### <a name="UNP-16" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> MCF-17: Incorrect Require Conditions | Type | Severity | Location | | --------- | ------------- | ----------------------- | | Logical Issue | Critical | [MasterChef.solL235](https://github.com/Yfswapfinance/YFBTC-Contract/blob/dfbf9558ca246e530016a0ccbe7c7ae12d08f51d/contracts/MasterChef.sol#L235),[L260](https://github.com/Yfswapfinance/YFBTC-Contract/blob/dfbf9558ca246e530016a0ccbe7c7ae12d08f51d/contracts/MasterChef.sol#L260),[L298](https://github.com/Yfswapfinance/YFBTC-Contract/blob/dfbf9558ca246e530016a0ccbe7c7ae12d08f51d/contracts/MasterChef.sol#L298),[L336](https://github.com/Yfswapfinance/YFBTC-Contract/blob/dfbf9558ca246e530016a0ccbe7c7ae12d08f51d/contracts/MasterChef.sol#L336),[L358](https://github.com/Yfswapfinance/YFBTC-Contract/blob/dfbf9558ca246e530016a0ccbe7c7ae12d08f51d/contracts/MasterChef.sol#L358),[L379](https://github.com/Yfswapfinance/YFBTC-Contract/blob/dfbf9558ca246e530016a0ccbe7c7ae12d08f51d/contracts/MasterChef.sol#L379) | #### Description: The require conditions in `pendingReward()`,`rewardPerBlock()`,`updatePool()`,`deposit()`,`withdraw()` is incorrect. This could be typos happened in the latest commit. Examples: ```Solidity require (address(pool.lpToken) == address(0), "MC: _pid is incorrect"); ``` #### Recommendation: We recommened to change the code as below: ```Solidity require (address(pool.lpToken) != address(0), "MC: _pid is incorrect"); ``` ### <a name="UNP-01" style="display:none"> </a><img src="https://svgshare.com/i/Pp1.svg" width="40"/> YFB-01: Transfer without `_moveDelegates` | Type | Severity | Location | | --------- | ------------- | ----------------------- | | Optimization | Critical | [YFBTC.sol L54，L62](#) | #### Description: Functions `transfer()` and `transferFrom()` do not transfer the `_delegates`. Token owners still can have votes even tokens transferred. #### Recommendation: We recommend to call `_moveDelegates` here. #### Alleviation: No alleviation. <div style="page-break-after: always; visibility: hidden"> \pagebreak </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.