OpenZeppelin Security Updates for February 2022

Simple Summary

Over the past month, the OpenZeppelin team has been primarily focused on completing its comprehensive audit of the existing Compound protocol. Over the course of this audit, a critical severity issue was identified and resolved. Since this critical issue was a result of a token integration bug, I am recommending that asset listings receive greater security scrutiny going forward and we will be working on new secure listing processes to be put in place. Finally, monitoring requirements have been proposed for community feedback before development begins on the Security Monitoring Dashboard.

Initiative Updates

Protocol Audits

Comprehensive Protocol Audit

As detailed in our last update, OpenZeppelin has been conducting a comprehensive audit of the existing Compound protocol codebase along with an audit of the cToken refactors that were developed by the Equilibria team. This audit started on January 24th and is wrapping up today after 5 weeks of work from 3 auditors.

As noted previously, a Critical integration issue affecting one of the cToken markets was found over the course of this audit and had to be resolved. This was done with guidance from the Compound Multisig and by working with the team behind the affected asset to patch the issue in the token contract without a need for changes to the Compound protocol. This incident was kept quiet from the wider community until its resolution to ensure public knowledge of the details would not lead to an exploitation of the issue.

We are in the final stages of releasing a full disclosure of the incident in coordination with all parties involved in resolving the issue to ensure the details are accurate. Since our audit report details the Critical issue, we will be releasing both the complete audit report and bug disclosure at the same time once both are ready. We will be publishing both documents next week which I will link under this post.

In the meantime, the community can read a summary of the audit findings below. We did not detect any high severity issues besides the one critical issue that has already been fixed. We did report numerous issues of lower severity along with suggestions to improve overall code quality which we hope can be incorporated into future protocol changes and upgrades.

  • Total Issues Found: 30
  • Critical Severity Issues: 1 (already resolved)
  • High Severity Issues : 0
  • Medium Severity Issues: 0
  • Low Severity Issues: 10
  • Notes & Additional Information: 19 (updated since some issues were combined)

Audit Backlog

We will be taking the next two weeks to document what we’ve learned and do team activities before resuming our next round of auditing on Mar 21st. We have still not reached a final decision on what our next proposal audit will be as what we’ve learned about asset listing risks has changed our initial plans.

Our backlog (now in order of priority) is currently:

  1. An Asset Listing Proposal (potentially FRAX)
  2. PR150: Oracle Improvement to Chainlink Price Feeds
  3. PR177: Enable Transfer ETH from Timelock
  4. Multi-chain Strategy Updates from Compound Labs (slide details)
  5. PR95: Compound supply cap

Our reasons for adding an asset listing to the top of our backlog is expanded on in the Security Advisory section to follow.

Security Advisory

Following up on our last Security Advisory update, we have now prioritized Asset Listing Security as our primary focus going forward due to what has been recently learned. We have learned by example that even if the Compound Protocol and the token contracts of each asset listed were independently audited, it does not guarantee that security issues will not arise when they are integrated together. We now know that new asset listings represent a higher risk than what much of the community might have previously believed.

As I stated in the FRAX proposal thread, we recommend that the community proceed with caution and reduce the number of asset listings made until we can define a better security process for catching integration issues. While some listing proposals such as MATIC are still moving forward, tokenholders should vote with an awareness that integration risks are always present. If the community decides to approve new assets before we are able to finalize a better security process (which will take at least several weeks), I would insist that the token implementations for any new listings be as standard and simple as possible.

Going forward, I propose that our next audit be conducted on one of the new listing proposals, such as FRAX. We would prefer that a future asset listing process not require manual audits of every proposal due to the backlog that would create for auditing protocol changes or vice versa. This initial asset listing audit would be done primarily to give our team an in-depth look at the process of listing assets and inform how we might be able to create a security checklist to catch potential integration bugs.

Our upcoming release of the Audit Report and Bug Disclosure should provide helpful details to the community so that everyone can learn from what’s been discovered. In the meantime, I recommend that any community members that are planning to list assets or want to contribute to a discussion about a more secure asset listing process start by watching one of our Security Workshop Recordings that covers the dangers of Token Integrations.

Security Monitoring

Using what we’ve learned from our comprehensive audit of Compound, we are proposing monitoring metrics and requirements for an initial version of a Security Monitoring Dashboard for the community to review below. The solution we are proposing will make use of Forta, Defender, and a publicly available analytics dashboard as detailed in our last update. Please note that the following requirements are still not finalized and are subject to change.

We recommend the following monitoring requirements:

  1. Monitor and log all events on the Compound protocol. This would expand on the same sort of activity provided by the Discord bot-feed and would include:
    1. Market activity such as borrows, withdrawals, supply, liquidations, etc.
    2. Governance activity including proposals and votes
    3. Other activity not listed but which could be captured in event logs
  2. Detect when COMP is borrowed in big quantities from Compound to provide early detection of minor governance attacks
  3. Monitor oracle pricing feeds to detect abnormal activity. Similar monitoring agents have already been built on Forta for Aave.
  4. Monitor the token contracts for listed assets to detect suspicious activity such as malicious upgrades or other potentially breaking changes
  5. Monitor COMP distribution amounts to provide early detection of distribution bugs such as the one caused by Proposal 62

While there are many more requirements that could be added over time, we want to settle on 5 initial requirements that will provide the community a valuable Security Dashboard and set the stage for future iterations. We welcome additional feedback and questions.

Our Request to the Community

We’ve already had an exciting two months working with the Compound community and we’re looking forward to what comes next. As we wrap up our first audit and begin working to implement what we’ve learned, we continue to ask the community to be open with their feedback and help direct our efforts.

To help us move forward, we ask the community for the following:

  1. Review our soon-to-be-published Audit Report and Bug Disclosure to learn more about how Compound’s security is positioned today and what could be improved in the future.
  2. Proceed with caution on all future asset listings and avoid unnecessary risks while we work to define a more secure listing process. Educate yourself on token integration risks and share suggestions for what a more secure listing process could look like.
  3. Provide feedback on our audit backlog priorities. We will likely move forward with auditing an asset listing next if we receive no other comments but we would like the community to weigh in on this decision since it will also impact the timeline for future protocol changes.
  4. Review our monitoring requirements to provide feedback and further suggestions. We would also encourage community members to learn more about how they can build monitoring agents using Forta which would allow future iterations of the Security Dashboard to be community-driven, rather than being fully reliant on OpenZeppelin.

Before I close out this update, I’ve got to give special mention to the auditors and all the OpenZeppelin team members that made our first DAO partnership audit a success. I also have to thank the community for their support over these first two months and most especially the Compound Multi-sig members for their guidance in resolving the Critical Issue that was uncovered. DAO-based security partnerships continue to be a new learning experience for the team and so we appreciate everything that’s been done by community members to help us succeed and make Compound more secure.

As usual, feel free to share your feedback below or reach out directly to me on Discord, Telegram or email:

Publication Updates:

6 Likes

@cylon What is the priority for a security review of adding MATIC as an asset?

We’re currently evaluating three asset listings: FRAX, RAI and MATIC for the next audit. We’re still exploring which would be the best test-case for us to inform how to create a security checklist for asset listings.

It’s important to note that the priority is not to audit the asset listing itself but to audit the overall process for listing assets on Compound. A security checklist and more formal listing process is the ultimate outcome of this audit that could then be used by all other asset listing proposers. Clearing the specific asset listing that we end up auditing would be more of a bonus since the goal is to create a process that doesn’t involve audits as a bottleneck going forward.

So we’re happy to get feedback on which asset the community would prefer that we prioritize but I would recommend selecting one of the riskier assets. This would provide more opportunities to test for edge cases that could lead to integration issues we would include in a security checklist.

3 Likes

Thanks for the thorough update and bravo on a very low-key handling of this critical bug. Looking forward to reading the audit and disclosure when it’s available.

1 Like

Here is the full Audit Report available as a PDF that details all 30 issues found: PUBLIC - Compound Comprehensive Protocol Audit Report.pdf - Google Drive

I’ll cover the report details on the upcoming Developer call and I’ll be posting a separate disclosure post on the Critical issue in a few hours.

2 Likes

Thanks for the update! What form would this security checklist take? Is this something that would be used by OZ or by the community when reviewing proposals?

1 Like

The goal would be something that can be used by proposal authors to check for integration issues. This would also be accessible by the whole community and would be used by OpenZeppelin auditors if we ever do more asset listing audits.

1 Like

Two updates that I’ll be sharing on the Compound Developer Call this week:

  1. An overview of the TUSD Disclosure published today
  2. Our next audit starting this week focused on the asset listing process
4 Likes