Yesterday, Hacken sent us back the remediation report following the 1st audit of Anystake.
This article is a layman’s terms explanation of the contents of the audit. It’s goal is not only to give you some details about Anystake results, but also to help you understand better how smart contract audits work in general.
If you are in a hurry, jump directly to the “Final Audit Results” part.
Protocol: Audit and Remediation
Before we start digging into the audit details, we want to explain a bit the timeline of a smart contract audit.
It’s basically done in 3 phases, but could be more if security issues remain after the second phase.
Phase 1: Initial Assessment
Where you simply send the finished version of your original code to the auditor. During this phase, the auditor will perform multiple analyses of the code for known vulnerabilities. In our case, it looked like Hacken even tried some of these tests on our live Testnet Rinkeby contracts!
Based on the outcome of the tests, the auditor provides a document detailing all the issues they found. The auditor can also provide specific recommendations to the team.
Phase 2: Correction
The team now needs to correct all the elements of the audit and to send back the final proposal to the audit company.
Phase 3: Remediation
Based on the corrections made, the Auditor re-audits the contracts and gives their final recommendation. Hopefully at this stage, every single error is cleaned and the contracts are secure. If not, the team will need to go back to Phase 2 and repeat the process. Many times, it causes launch delays and additional audit expenses if the contracts are sent back again.
Structure of the Audits from Hacken
During each step of the process, Hacken provides a PDF report which lists the functions they tested, structure of the smart contracts, and any security issues they could have found.
Hacken severity issues is structured as following:
Now let’s jump into our audit results!
Anystake 1st Audit
Let’s face it… it was not great.
And I (Defiat_Dev1) take the full responsibility for the low score we got. I pushed too hard on Quant to launch fast.
“Fast, secure, or cheap development: pick 2 out of 3."
Our team is small, we are cheap by default and cannot afford to be expensive. Now we know we have to take the time to push something of quality.
(Add to that that, sometimes, life is not always smooth sailing…)
This makes even more sense when you look at the code: AnyStake is not a fork or an existing proven protocol. It’s new and it’s complex. It involves a 2-token structure (DFT + DFTPv2), token farms, a rewards vault, and the mighty Regulator.
Side note: If you look on the Hacken audit website https://hacken.io/audits/, you will see that a vast majority of audits concern ERC20 (proven secure code by OpenZeppelin) or forks (Sushi winning the race as being the most forked). Genuine code is a rare concept in the space since verifications on Etherscan.io ask for the code to be public and open source.
Here are the details of the security issues distribution:
The bad news was: You cannot ship code that has High or Critical severities. It’s a NO-NO. Medium and Low in general need to be analyzed, sometimes it’s more about having clean code than having a loophole in the protocol.
The good news was, it’s the 1st try. It doesn’t have to be perfect because we can correct elements of the code for remediation.
Final Audit Results
In the next chapter, we will dig into each remark and explain how we remedied the associated issue.
But 1st, let’s look at the FINAL RESULTS:
It’s a great score. The team (kudos to Quant) not only managed to clean all the issues, but the final score is also a representation of the overall quality of the protocol. If most ERC-20 can get a “well-secured” score, it’s rare for farms and more complex protocols to be that close. Most stand around “Secured”.
Detailed Audit Results
For the curious only. We’ll start with Critical and go lower in the severity grade, and explain each of the Audits’ remediation.
1. AnyStakeBooster was not ready (see my remark above on the 1st audit).
As a reminder, it’s now called “Preferred Pools” or “VIP Pools”. It is the solution that will open some pools ONLY if you locked (staked) some DFT tokens in the DFT Pool, pool. [SOLVED].
2. Anybody could call addReward() on the Regulator. Meaning that anybody could add the DFT reward counter on the Vault, even if no DFT was transferred. This issue could cause AnyStake and Regulator to throw an exception on every function if abused. Quant added a modifier so this function can only be triggered by the Vault as intended, just like AnyStake. [SOLVED].
3. This one was a loophole: The standard withdraw() function lets you take your stake (minus a fee), but also triggers many other functions like buybacks and rewards distribution: if one of these functions would have a problem, the the whole withdraw() would revert and stakes would remain stuck in the protocol (no bueno…).
So we had setup emergencyWithdraw() that allows users to exit their stake without triggering a buyback nor rewards distribution in case of Black Swan events. If anything would happen on the buybacks or rewards side, this function would still work.
But… it became clear that people could use this to exit WITHOUT paying the 5% exit fee. So Quant implemented a simple withdrawal fee on the function emergencyWithdraw(), without the buyback function. This fee is simply transferred to the Vault, where admins can then manually perform buybacks in the case it were ever to be used. [SOLVED].
4. Buybacks being frontrunned. Not a code issue, but a trading possibility.
To frontrun a buy trade, you need to know first that this buy trade will impact the price significantly. Then, you send 2 transactions: one buy right before the call (reading Eth memory pool of tx and loading on gas to go 1st), and one sell right after. This “sandwiches” the original buy trade and allows frontrunners to profit from slippage settings.
The bot bought at 100, the frontrunned buy trade made the price go to 104, bot sells at 104 (minus slippage). This yields 4% profit minus fees minus slippage.
But not for DFT… because each of these trades would cost the bot 2.5%, hence its margin would be 4%-2.5%-2.5% minus fees and slippage: Negative.
We explained to Hacken that the nature of the DFT token (2.5% fee on trades, as decided by the community) is a deterrent for frontrunning bots and they considered this as [NOT AN ISSUE].
5. Our code had the Uniswap router and factory hard-coded. So if these guys change it, not only the whole DeFi space would break and signal the end of the world… (as literally trading would become impossible using the existing pools), but Anystake would not be able to perform buybacks and some underlying functions would not work anymore (see #3).
Simple fix: make these addresses a variable, and create a function to change them if needed (Admin only). [SOLVED].
6.We include a function in the Vault to find the price of a given token or LP token on Uniswap. The current implementation of the function would be vulnerable to a flash loan attack on LP Token ONLY.
Flash Loans basically let you borrow ETH and reimburse it in the same transaction. The transaction only works if you end up having enough ETH to reimburse at the end of the call.
A . Borrow ETH from Aave, DyDx, etc.
B. Use ETH to buy tokens to pump price
C. Perform a malicious attack
D. Sell tokens bought in B. to dump price
E. Give back ETH to loaner +a small fee
It looks like a cool way to make free monies, but we only use the function to find the LP token price on the frontend website; not in the code. We do use this function to find the price of DFT and DFTPv2 in Regulator, but we use the getReserves() method which returns the average price of the token over the last 15 blocks, so it is impossible to perform this type of attack. [NOT AN ISSUE].
7. AnyStake, like many farms relying on fee-on-transfer tokens, requires a claim to be done at every interaction to avoid math inconsistencies or people trying to game the system (i.e. deposit 1 token, wait 3 month, then deposit 100 tokens. Suddenly, your rewards show as if you had staked 100 tokens during 3 months). Safety here comes at the gas price of an additional claim() function. [NOT AN ISSUE].
8. This one was a bit tricky. Gas optimizations require your code to be as efficient as possible. Our solution was to change the way DFT rewards were managed such that reward counters are kept on AnyStake and Regulator, while keeping all DFT rewards on the Vault. This allowed us to remove 2 transfer calls per contract interaction, saving ~100k gas per interaction [SOLVED].
9. claimAll() simply was looping on all the pools and performing a claim(). The issue is: if you don’t have a stake, claim() reverts.
We went simple and decided to remove the claimAll() function. [SOLVED].
10.Not entering into details here on why it’s recommended to lock your compiler version, but this one was a simple fix: locking the solidity pragma version in the code to 0.6.6, the same version we compiled the DFT and DFTPv2 tokens with. [SOLVED].
11. Comment was corrected. This had no impact on the contract, but it shows how deeply Hacken gets into details. It simply highlights the quality of their smart contract audits in general. [SOLVED].
As said, the 1st iteration wasn’t great. (I took the walk of shame for pushing on the team too hard…). But everything was solved in the end thanks to our master at work, Quant.
All the issues were fixed for the secondary audit.
We are now proud to have SECURE contracts and can move on and deploy the final version of AnyStake on-chain.
— The DeFiat Team