Code4rena Audit Competition: Post Audit Wrap-up
An overview of findings and remediation from our time in Code4rena's audit competition.
This past October, we put our contracts through a true test of their metal in a round of Code423n4’s Code4rena!
A group of 15 Code4rena Wardens took to the arena and ravaged our contracts in search of any issues or recommended optimizations. With $75,000 up for grabs, there was no holding back as the Wardens competed for their share of the prize.
The Warden’s investigations culminated in a comprehensive review of our smart contract logic, as well as a look at a any technical oversights, and suggested gas optimizations.
For a complete review of Code4rena’s findings see their report. For a summary of major findings and our remediation just read below!
“High”-Risk Findings:
1. [H-02] Taker is charged fees twice in ‘exitVaultFillingVaultInitiate’
In the case where a user may be selling their nTokens directly to another user, there was a mismatch in codebase versioning that was missed in our internal reviews, leading to a duplicate fee in one method.
This was remediated by ensuring the Swivel codebase implemented fees in a consistent way across all methods.
Remediation: Link
2. [H-03] TransferNotionalFrom
doesnt check from != to
An issue common across a number of codebases that has led to a number of large hacks including the recent 30m MonoX hack. In many cases one must ensure two vaults owned by the same user cannot interact with one another.
When this is not implemented, a vault can send currency (in our case notional) to itself and overwrite an intended zeroed out storage from the sender with the increased value of the receiver. This effectively allows one to mint itself currency.
The fix from this is simple, add a revert if a user is sending notional to themselves.
Remediation: Link
3. [H-04] Return value of 0 from ecrecover not checked
We actually contested this issue as there is no impact.
The potential vulnerability is that users can create signatures that fail and resolve to the 0 address.
That said, the 0 address cannot set any approvals to our contract, so there was no contract vulnerability, only DDOS potential, which we already account for on our orderbook.
Remediation: None required.
4. [H-01] Unsafe handling of underlying tokens
While we also did not believe this to be an issue (at least high risk), we still acknowledged and remediated it as the judges suggested.
In cases where a token does not properly return true/false from its transfers, there can be unexpected contract execution.
That said, we act as admins who approve tokens, therefore there was no present vulnerability. Nonetheless, we have still added safeTransfer to provide users an extra guarantee of safety.
Remediation: Link
Notable Medium Risk:
1.[M-03] Previously created markets can be overwritten
While admin permissions gate access to market creation, an admin error in creating markets could have led to the recreation of an already created market, and a resulting loss of funds.
In order to prevent admin error from leading to any loss of funds, we have increased our market creation validation in a variety of ways, one being the prevention of overwritten markets.
Remediation: Link
2.[M-05] Implementation for initiatezcTokenFillingzcTokenExit is incorrect
A minor mismatch in our fee implementation versioning for two methods led to the incorrect fee being taken in each method.
To remediate this, we ensured that the fees maintained a constant structure across our entire codebase.
Remediation: Link
3.[M-04] Fee-on-transfer underlying can cause problems
Certain tokens behave abnormally / do not completely conform to the full ERC-20 standard.
Tokens that have a fee on any transfers, or potentially can activate a fee on transfers (i.e. Tether), could pose a security risk as our contracts cannot account for any transfers out of Compound or any other partner integration.
Our remediation was to add a pause button / circuit breaker in order to ensure limited vulnerability to Tether markets.
Remediation: Link
Additional Findings:
Beyond the highlighted issues, the Wardens identified:
18 Low-Risk:
Input validation
Maximum Fee
Event clarity
Incorrect code comments
20 Non-Critical:
Code clarity
Comment clarity
Style issues
28 Gas-Optimizations:
A variety of gas optimizations that led to a roughly ~20% reduction in execution costs
Conclusion
Our time in the Code4rena led to an exhaustive review of our codebase and a significant rework of our implementation to reduce execution costs roughly 20%.
While we can never completely guarantee the safety of user funds, we are committed to being as open with our review process as possible. Code4rena’s open audits serve to ensure as many security experts have their eyes on our code as we can.
With a significant reward pool, the open audit process has proven to be more cost effective and produce better results than most contracted auditing firms. Given the performance of the wardens in our most recent audit (in comparison to our previously contracted audits), we plan to have the Code4rena Wardens review any significant codebase changes going forward.
What Next
Come on by our discord and earn your spot as a member of #the-chairmen!
Keep an eye out for our educational content contest! We’ll be looking for the most creative and informative folks in our community to help create the best techsplanations and educational propaganda possible!
We’re also in search of a few talented and creative folks to join our team! If you think you might fit in, regardless of your skillset (devs, artists, operations, whatever!), feel free to reach out directly or apply!
About Swivel Finance
Swivel is the decentralized protocol for fixed-rate lending and tokenized cash-flows.
Currently live on Rinkeby, Swivel Finance provides lenders the most efficient way to lock in a fixed rate as well as trade rates, and liquidity providers the most familiar and effective way to manage their inventory. Through our exchange, traders can take on positions with minimal slippage, even in low liquidity markets.
Website | Substack | Discord | Twitter | Github | Gitcoin | Careers