During the time frame of February 13th to 14th, 2023, Shellboxes was tasked by Sukiyaki’s team to assess the security of the Sukiyaki system. Our examination involved a systematic evaluation of the potential security risks related to the utilization of smart contracts. Our approach spotlighted any mismatch between the code and design of the smart contracts and proposed further enhancements to optimize the code. Our findings indicated that despite numerous security and performance problems, there is still room for improvement in the current version of smart contracts.
Sukiyaki uses custom built AI to route for best prices on chain and cross chain for users, thereby gaining points ahead of the competition over 65% of the time on Ethereum, Binance Smart Chain, Arbitrum and Polygon.
Sukiyaki works like a regular DEX aggregator, but there are some more advanced touches put into the mix to ensure that their DEX is more efficient than the competition.
First, a user requests a swap, and the Sukiyaki queries over 50 DEXs for that same trade, including Uniswap, Sushiswap, 1inch, Paraswap and many more DEXs, and even searches for routes on other chains including Arbitrum, BSC, Avalanche and Polygon for the same exact swap to see if better prices exist off-chain. When these DEXs return their quotes for the swap they requested on the user’s behalf, their AI sorting algorithm sorts the prices by the best and takes a lot of things into consideration, such as the gas fees, time to completion and the number of approvals required.
The ShellBoxes team was faced with the challenge of balancing various important factors such as speed, efficiency, accuracy, and scope when conducting their security testing. To address this challenge, they decided to use a combination of both manual and automated testing methods.
Manual testing was considered essential for detecting errors in logic, procedure, and execution. It also played an important role in verifying the protocol’s invariants from the business logic to ensure that they aligned with the code implementation. This level of manual scrutiny was deemed necessary to ensure that any potential security threats were identified and addressed.
On the other hand, automated testing was utilized to broaden the scope of the security assessment and to quickly detect any code that did not comply with the established security best practices. Automated testing allowed the ShellBoxes team to cover a larger surface area and detect any potential security issues at a much faster pace.
In this way, the team was able to strike a balance between speed, practicality, accuracy, and scope, thereby ensuring that the security evaluation was comprehensive and thorough.
The smart contracts under review were implemented for a custom ERC20 token project, which has been designed to distribute fees from transfers to multiple parties, including the development team, marketing team, and liquidity pool. The project is intended to incentivize and reward various stakeholders involved in its development and promotion, while also providing liquidity to support the token’s overall value.
Overall, the smart contracts implemented for this project were found to be well-designed and properly constructed. However, upon closer inspection, several vulnerabilities were discovered in their implementation that could pose risks to the project’s security and integrity. These vulnerabilities include two high-severity, seven medium-severity, and four low-severity issues, which require immediate attention to mitigate potential risks.
One of the key vulnerabilities discovered in the smart contract implementation is related to the “swapBack” function, which is responsible for distributing contract funds collected from transfer fees to the devAddress, marketingAddress, and liquidity pools. The function is missing a critical check over the return value of the call function used for transferring ETH value. As a result, if the call fails, the transaction will not revert, and the variables “tokensForDev” and “tokensForMarketing” will be set to zero, causing the devAddress and marketingAddress to permanently lose those funds.
Another significant vulnerability identified in the smart contract implementation is related to the “swapBack” function, specifically the “tokensForBurn” variable. This variable is supposed to be used for burning tokens when its value is greater than zero. However, when the contract balance is lower than the value of tokensForBurn, the variable is set to zero without burning any tokens. This inaccuracy in the contract logic poses a critical vulnerability.
Medium-severity vulnerabilities were identified in the smart contract implementation. The “transferOwnership” function has a vulnerability that allows the old owner to retain exclusions while the new owner does not have them. The “transferForeignToken” and “withdrawStuckETH” functions provide the contract owner with complete control over the contract’s funds, creating a centralization risk. The lack of a fee limitation in the “updateBuyFees” and “updateSellFees” functions can compromise the contract’s structure. The overuse of the “onlyOwner” modifier creates a centralization risk that can lead to the abuse of power. The presence of modifiable variables creates a race condition vulnerability. Finally, the centralized token allocation in the constructor creates a significant risk for the contract.
The low-severity vulnerabilities discovered in the smart contract implementation are as follows:Firstly, the approve function in the standard ERC20 implementation contains a race condition vulnerability, where a spender can manipulate the approval transaction to move the current approved amount from the owner’s balance to the spender, potentially exploiting this vulnerability to double dip on the approved amount. Secondly, the use of transfer() and send() to transfer Ether, although recommended for preventing reentrancy attacks, may break deployed contracts due to the gas repricing of opcodes. Thirdly, the contract implements an early buy penalty for bots and snipers, which is not documented, potentially causing a loss to legitimate users. Lastly, the owner can renounce ownership using the renounceOwnership function in the Ownable contract, which may result in some owner-exclusive functionality becoming unavailable if the ownership has not been transferred before.
The contract lacks several best practices related to code quality and gas optimization. The lack of documentation of the state variables makes it hard for readers to understand the intended behavior, and the redundant call to transfer ownership in the constructor should be removed. Additionally, the initialization of variables with their default value is unnecessary and can be omitted. The use of revert statements instead of require statements is recommended for better gas optimization. The prefix operator ++ i should be used instead of i++ as it is less gas-consuming. The order of emitting events and initializing variables should be corrected in renounceOwnership and transferOwnership functions. Lastly, the require statements in the returnToNormalTax function are redundant since the fee values are hard-coded, making the verification of the values unnecessary.
In this case study, we have analyzed a smart contract and identified several issues related to security and best practices. These issues include missing return value checks, a racing condition vulnerability, the use of transfer() to transfer Ether, undocumented early buy penalty, the ability to renounce ownership, lack of documentation, redundant code, and inefficient use of Gas… To address these issues, we have provided recommendations such as avoiding transfer() and instead using call(), documenting the code, removing redundant code, and using efficient Gas consumption techniques. By addressing these issues, the smart contract can become more secure, reliable, and user-friendly, which can improve the trust and confidence of users in the system. It is crucial to follow best practices and ensure that smart contracts are thoroughly audited to prevent vulnerabilities and ensure the safety of users’ funds.
The Sukiyaki Finance team has decided to acknowledge our findings and not proceed with the fixes due to the fact that the contract is renounced and the liquidity pool is locked. The contract renouncing is based on their community’s demand post the Fair-launch with the goal of building their community’s trust.
© 2024 ShellBoxes All Rights Reserved
