One last audit

1

Pooltogether v5 is scheduled to launch next Thursday, Oct 19th. This new version is inspired by Hyperstructures The key properties such as unstoppable and permissionless are commendable.

At that date, launch means deploying various smart contracts both on Ethereum mainnet and L2 Optimism - please correct me if I am wrong in the list below

  1. On mainnet, 4 smart contracts from this repo https://github.com/GenerationSoftware/pt-v5-draw-auction/tree/main/src. See Ethereum goerli https://dev.pooltogether.com/protocol/next/deployments/testnet/#ethereum-goerli

  2. On Optimism, see https://dev.pooltogether.com/protocol/next/deployments/testnet/#optimism-goerli

  1. Contracts for liquidations

  2. Contracts for claimimg

  3. Contracts for booster (should be postponed in my opinion)

These contracts have been audited in July and August in code4arena GitHub - code-423n4/2023-07-pooltogether and GitHub - code-423n4/2023-08-pooltogether. One other audit by a private firm has also been done.

A proposal from hats.finance has been made recently for another audit

Now the solidity code of the contracts above has seen changes since the relevant commits of the audit. Mainly because of the security findings from the audit competition. But for other reasons also. Some identified medium vulnerabilities still remain AFAIK, see eg M-22 Unmitigated · Issue #96 · code-423n4/2023-08-pooltogether-mitigation-findings · GitHub

I believe pooltogether deserves its contracts to be audited one last time, on the final commit bedore deployment for each repo.

Timing is short until launch date next week, postponing is difficult, big party with world class DJs is planned.

Money seems short, apparently no help from Grant team may be asked but we can call for poolers to lend the money. Or use retroactive grant maybe. Or something else. Anyway no money is spent if there are no findings!

With swift action and partnering with hats.finance or not, we can do it!

We may probably reduce the scope of the audit to just contracts from points 1. and 2. above. I will try to compute some data on the number of changes since last audit in the associated solidity files very soon.

Question for @sombrero : do you require the ERC20 tokens to your vault to be something specific? from one address? How much money for the vault do you need - I believe it is a proportional to the number of solidity line? How much time of a solidity engineer from pooltogether do you expect for the setup of the audit on the one hand and the classification of the possible findings on the other?

2 Likes
2

Thanks for starting this discussion, @sliponit . Smart contract security is very important and audits help to instill confidence in depositors and as such, we should always ensure that the code meets the standards of the community. I will ultimately defer my opinion on this matter if it comes to vote.

The two audits from code arena and the additional private audit were extremely helpful in finding bugs, refining the design of the system, and improving gas optimizations. Although there were many critical issues that were brought up and addressed, many of the issues that these audits bring up are not critical and the majority are opinionated formatting suggestions and extremely small gas optimizations. These kinds of issues take a long time to go through with very little reward for the protocol. We do not have the time to go through hundreds of “issues” that say we should move a declaration further down in a file to make the file look better (see example report).

That being said, this proposal from Hats would be more appealing to me if they can agree to only report high and critical findings that affect funds or protocol configuration. However, even if they agree to do so, I am not certain that this would be the most effective way to spend treasury funds. Having a robust bounty system is also an efficient way of finding new vulnerabilities and regardless of what the community chooses to do with this audit, a bounty system for V5 should definitely be set up so that white hat hackers can be rewarded for any vulnerabilities they bring to our attention. Bounty systems also help protect against future vulnerabilities that may be introduced through the use of new vaults or prize hooks.

In conclusion, if an additional audit is something the community wants to see, then I am for it with the exception that only high and critical findings are paid for by the treasury.

4 Likes
3

Tx a lot @trmid for your reply.

About only medium and critical, I will let @sombrero respond for hats.finance. But on their system, auditors need to report on-chain and pay gas for that… This limits the number of “findings” compared to say code4arena, see Audit Competitions - Hats.finance

About bounties, yes it is great and we should definitely do them. But the findings will be once poolers already deposited lots of money…

2 Likes
4

Thanks for the proposal @sliponit. I’m open to this idea, though it is last minute and funding could be hard to manage in such a short time period. Having a better picture of what the potential costs are would be good. It allows for a cap?

If I remember correctly with V4 we paid out two bounties for vulnerabilities pretty shortly after launch. At least one I think was caused by mitigations made between audit and launch. We redeployed the production launch as a result of the 1st report. @midpoint can you speak to your perception of risk in the mitigations and changes to the contracts since the audits were completed? Have the auditors reviewed any of the mitigation work? Are there other changes made that were not mitigations that could be risks to funds?

I agree with @midpoint that it would be more appealing if we could limit such an audit to findings that are vulnerabilities to funds (deposits and yield).

I see this proposal and posting bounties as entirely separate. We should definitely have security bounties. I thought that was part of Generation Softwares approved budget?

2 Likes
5

The GS team focused on the bigger issues first when it came to mitigations, and the mitigation report for the first Code Arena audit is publicly available here. For the issues that were determined to be not fully mitigated, GS did further mitigations to address the concerns in the report.

There were many low risk and Q/A issues from the reports that were addressed during the beta and after the mitigation review. These issues focused on gas savings, code legibility, and system efficiency. I cannot guarantee that there were no additional vulnerabilities introduced in these changes, but I will say that every commit underwent peer-review and intense scrutiny in-house.

There were also a couple critical findings that none of the audits found, but were found in-house at GS and were addressed. We should remember that PTv5 is a complex protocol and it is difficult for an individual or firm to fully understand all the nuances of the protocol in a short amount of time. Hosting a 5 day audit with people that have never seen the thousands of lines of code before may bear less fruit than having a robust bounty system that allows bigger payouts to people who diligently look over the code over a longer span of time.

I must humble myself and say that there is no guarantee that I (or any auditor) have found every possible issue in the protocol. At the end of the day, audits and bounties are about risk management. Nothing is guaranteed to find everything, but we should always do our best to try.

This is coming from my perspective of looking at this code non-stop for the last 6 months. I understand that things will look different from someone who is not as familiar with the codebase, and that others may feel more comfortable if there is another audit. So ultimately, I will support whatever the community decides to do.

2 Likes
6

I believe we have done sufficient due diligence for the security of the contracts.

We have conducted three audits with two separate auditing firms.

The Code Arena audits covered the entire code base. The 0xMacro audit covered the Prize Pool and Twab Controller. Both firms converged on very similar results for the common code, which gives me confidence that we’ve tackled all of the obvious issues.

During the audit, the auditors were struck by the complexity of the code. It was a big challenge for them; this is not a simple protocol. Code Arena has dozens of auditors and spent one week with the code. 0xMacro had a group of three experts, and spent a month focused on the code.

It is unrealistic to expect auditors to get up-to-speed in a short timeframe on a complex code base that took 6+ months to write. I’m very impressed with what they accomplished, but I also understand their limitations. It takes time for people to gain a full understanding of our codebase.

We are in the process of writing documentation that futher explains the mechanisms and rationale behind the protocol. This material will help people understand, but it will take time.

We need an on-going bounties program to ensure that researchers have an incentive to continue to pursue issues within our codebase. I am working on an engagement with Immunefi, who are the clear market leaders here. This will help promote PoolTogether to security researchers.

I don’t think a short 5-day bounty program will be worthwhile. I like the idea of Hats, but this is not the right move at the moment.

I’ve been designing, building, and managing security for the PoolTogether protocol for over four years. I’m making this decision for GS because I think we’ve had the right balance of security, expediency, and cost. GS is going to move forward.

That being said, the community is free to pursue anything they wish!

4 Likes
7

I strongly recommend holding an audit competition on the final commit, instead of a 3-month old commit for some contracts. I believe it’s important to audit the code that will actually be deployed to the blockchain, not an outdated version.

If you’re a member of the community and would like to see this happen, please let us know! I’d also love to hear your thoughts on why or why not you think this is important.

1 Like
8

I think this is becoming the mantra of v5. It’s complex. In the contracts, in the docs, in conversation - it’s complex.

I would agree that a 5-day audit will likely not be worth the while. However, what is the bounty budget after launch? Is this something GS has in their budget or is it expected that the protocol back the bug bounties?

1 Like
9

Finally got around computing that data for contracts in points 1 and 2 above :slight_smile:

Screenshot from 2023-10-15 08-37-05
Screenshot from 2023-10-15 08-37-051900Ă—676 106 KB

10

Hello, thanks once again for tagging me.

The token can be any ERC20, whether on the mainnet or on Arbitrum, Polygon, Optimism, or Gnosis.

Budget allocation is determined by either the committee or the project itself. Our recommendations are based on factors such as the number of SLOC and the complexity of the code, which are provided by Hats’ internal auditors.

As for the setup, we can complete it in less than an hour, working in collaboration with our onboarding team.

Classification of submissions depends solely on the knowledge of the development team. If they are familiar with the code, labeling submissions can be done within minutes.

I hope this answers all your questions.

1 Like
11

Bug bounty programs are undoubtedly valuable, but they’re most effective when implemented after you’re confident in your initial audit. Due to the nature of these programs, the cost of a discovered vulnerability when funds are at risk can be significantly higher than investing in another audit round.

As for Hats’ audit competition and severity levels, you can opt to launch a campaign focusing solely on High and Medium issues. Freelance auditors will be more motivated to scrutinize your code if the potential rewards are substantial.

Given that confidence is paramount in this space, Hats’ mechanism provides the safety net you require. It operates on a straightforward principle: pay for results. If there are no results, there’s no payment. It’s a win-win solution.

1 Like