Skip to content

Klevoya WASM Smart Contract Fuzzer grant application#420

Merged
semuelle merged 2 commits intow3f:masterfrom
klevoya:master
Jun 4, 2021
Merged

Klevoya WASM Smart Contract Fuzzer grant application#420
semuelle merged 2 commits intow3f:masterfrom
klevoya:master

Conversation

@mtabz
Copy link
Copy Markdown
Contributor

@mtabz mtabz commented May 21, 2021

Grant Application Checklist

  • The application template has been copied, renamed ( project_name.md) and updated.
  • A BTC or Ethereum (DAI) address for the payment of the milestones is provided inside the application.
  • I have read and acknowledged the Terms and Conditions.
  • The software delivered for this grant will be released under an open-source license specified in the application.
  • The total funding amount of the project is below USD $30k for initial grant applications and $100k for follow-up grants.
  • The initial PR contains only one commit (squash if needed before submitting your PR).
  • The grant will only be announced once the first milestone has been accepted.

@alxs
Copy link
Copy Markdown
Contributor

alxs commented May 21, 2021

Thanks for the application @mtabz. We'll look into it as soon as possible.

Copy link
Copy Markdown
Contributor

@Noc2 Noc2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the application. You should potentially also take a look at the following grant project: #9 or here: https://github.com/pventuzelo/wasm_runtimes_fuzzing. However, this grant was for wasmi/wasmtime and not a polkadot or contract runtime. Why do you think fuzzing a runtime will be useful? Isn’t it possible to ensure through the statistical guarantees of rust that the runtime won’t crash. Did you discuss this with parity directly, since they developed the module?

@Noc2
Copy link
Copy Markdown
Contributor

Noc2 commented May 21, 2021

Let me also ping @athei here, who is working on the contract module.

@Noc2 Noc2 self-assigned this May 21, 2021
@Noc2 Noc2 added the changes requested The team needs to clarify a few things first. label May 21, 2021
@mtabz
Copy link
Copy Markdown
Contributor Author

mtabz commented May 21, 2021

Hello @Noc2, yes we have seen the project #9, however as you have mentioned that project was focused on fuzzing wasmi and our goal is different.

Why do you think fuzzing a runtime will be useful?

We're not planning on fuzzing the runtime in this grant. Rather our focus is on fuzzing the smart contracts that will be deployed on the contracts pallet. (A good example from the Ethereum ecosystem of the kind of tool we want to develop here is https://github.com/crytic/echidna which is an Ethereum smart contract fuzzer.)

We primarily plan to use fuzzing to identify smart contract logic bugs. With regards to memory safety, you are indeed right that with Rust as a smart contract programming language there should be fewer memory safety issues found through fuzzing the smart contracts. However, there are other smart contract programming languages in development by other teams that do not offer the same compile-time memory safety guarantees (e.g. https://github.com/patractlabs/ask which is AssemblyScript based). In this way we hope to help smart contract developers identify a wide range of security issues in their smart contracts.

Did you discuss this with parity directly, since they developed the module?

We have not discussed this with parity. We'd be happy to do so if you can suggest a suitable person to contact?

@athei
Copy link
Copy Markdown

athei commented May 25, 2021

As the de-facto maintainer of the contracts pallet I would welcome the addition of a fuzzer. However, after reading the proposal I still have some open question:

  1. Given I understand your proposal correctly your goal is to fuzz pallet-contracts( i.e find bugs in this piece of software). However you propose to not fuzz against the real software but your own implementation in order to reach an execution speedup (transpiling to C++). How are you finding bugs in pallet-contracts when you execute your input against another software?
  2. Do you think fuzzing a software written in pure Rust that tries to avoid panics at all costs is a worthwhile endeavour? I expect most bugs to be logic flaws or other high level bugs that cannot be found by a fuzzer which is just looking for crashes.

@mtabz
Copy link
Copy Markdown
Contributor Author

mtabz commented May 25, 2021

Hi @athei, thanks for the response.

It appears that the grant proposal could benefit from some additional clarification around the intention as this seems to be unclear.

To clarify:

  1. Fuzz target: The proposal is not to fuzz pallet-contracts. We assume that this piece of software is largely bug-free. However (and please do correct us if we are mistaken), the contracts pallet will be used to execute smart contracts written by 3rd party developers, and these smart contracts may contain a variety of bugs. Our intention is to fuzz those smart contracts, which is why we believe we can execute the smart contracts against our own model of the contracts pallet.
  2. Logic bugs are the primary goal: yes we completely agree with you that most bugs will be logic bugs. We can see from other blockchain ecosystems that the majority (all?) of smart contract hacks that take place are a result of logic bugs. This is why the fuzzer we are developing will not only detect crashes; in the second half of the grant we plan to implement techniques that allow the fuzzer to detect common smart contract logic bugs.
  3. Memory corruption errors are a secondary goal: as I noted in my response above to David, ink! (and by extension Rust) is not the only language that smart contracts may be written in in the future, as we start to see other smart contract languages being developed by the community (like Ask!) that are not based on Rust. Smart contracts derived from these non-Rust languages may exhibit memory corruption errors that a fuzzer may catch.

@athei
Copy link
Copy Markdown

athei commented May 25, 2021

Thanks. Now it makes more sense to me. Would be nice to have some clarifications in the proposal that mentions what it is not (fuzzer for pallet_contracts, fuzzer for the wasm engine) and maybe mention existing work in those other fields by other teams so we know where it fits in.

That said, rebuilding the complete logic and keeping up with changes to pallet_contracts seems to be some non trivial work. Do you think you can get rid of much of the logic or mock it somehow (like gas metering) to keep your implementation simple?

Maybe you could add your ahead of time compiling engine as a backend to sp_sandbox so that you don't have to keep your implementation in sync with the production module. Otherwise I am worried that the fuzzer gets outdated quickly. However, I am not sure if this is even feasible. Maybe as a follow up grant.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jun 2, 2021

CLA assistant check
All committers have signed the CLA.

@mtabz
Copy link
Copy Markdown
Contributor Author

mtabz commented Jun 2, 2021

@athei we have updated the proposal with your feedback on making the project goal more explicit and included details of similar work. Let us know if there are still items we should add to make it clearer.

With regards to future maintenance, the plan is to have API level compatibility with the contracts pallet and underlying logic being mocked as much as possible. In this way we hope to reduce the maintenance to a minimum.

We are not overly familiar with sp_sandbox, but agree that we can perhaps investigate this in a follow on grant.

The latter two points have also been added to the grant proposal.

Copy link
Copy Markdown

@athei athei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me from a technical standpoint. A fuzzer would be a great supplement to the new offchain testing environment that the ink team (cc @Robbepop) is working on which will be used for manual tests.

Copy link
Copy Markdown
Contributor

@Noc2 Noc2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating this. Also thanks a lot @athei for helping review the application! I’m happy to go ahead with it.

Copy link
Copy Markdown
Contributor

@alxs alxs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very happy to support this as well. Good luck with the development!

@semuelle semuelle merged commit 3394132 into w3f:master Jun 4, 2021
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jun 4, 2021

Congratulations! As part of the Open Grants Program, we want to help winning teams acknowledge their grants publicly. To that end, we’ve created a badge for projects that successfully delivered their first milestone. Please observe the foundation’s guidelines when making any announcements; in particular, don’t announce the grant publicly before you've completed at least the first milestone of the project.

At that point, we will be happy to collaborate on an announcement about the work you’re doing. Please get in touch with us at grantspr@web3.foundation in case you're interested (at least two weeks notice is preferred).

@alxs
Copy link
Copy Markdown
Contributor

alxs commented Oct 7, 2021

Hey @mtabz, care to share a quick status update, along with an ETA if possible? We were expecting to receive your first delivery a bit over 2 months ago. As long as you're actively working on the project, delays aren't a big problem, but please keep us in the loop.

Note that if we don't hear from you within the next two weeks, we'll assume you're no longer interested and terminate the agreement.

chrisli30 pushed a commit to AvaProtocol/W3F-Grants-Fork that referenced this pull request Oct 8, 2021
* Substrate WASM smart contract fuzzer proposal

* Clarified grant goal and similar work. Updated license.
@mtabz
Copy link
Copy Markdown
Contributor Author

mtabz commented Oct 20, 2021

Hi @alxs apologies for the delay. I can confirm that we are still working on this project. The reason for the delay is that we started working on it a bit later than envisaged (we were not expecting at the outset for the grant to be approved that quickly!).

ETA is in ~1 month.

@alxs
Copy link
Copy Markdown
Contributor

alxs commented Oct 21, 2021

@mtabz works, no worries and thank you for the update! :)

@alxs
Copy link
Copy Markdown
Contributor

alxs commented Jan 7, 2022

Hey @mtabz, any news?

@alxs
Copy link
Copy Markdown
Contributor

alxs commented Jan 20, 2022

@mtabz note that since we haven't heard from you in a while and given the delay, if we don't hear from you in the next two weeks we'll assume you're no longer interested and terminate the grant.

@alxs alxs mentioned this pull request Feb 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changes requested The team needs to clarify a few things first.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants