Stream: git-wasmtime

Topic: wasmtime / PR #8488 cargo-vet: Exclude fuzzing-only depen...


view this post on Zulip Wasmtime GitHub notifications bot (Apr 26 2024 at 07:54):

jameysharp opened PR #8488 from jameysharp:no-vet-fuzzers to bytecodealliance:main:

We can't meaningfully audit the other WebAssembly implementations that we use for differential fuzzing, such as wasmi and especially v8. Let's acknowledge that the effort to do so is not practical for us, and focus our vetting efforts on crates that developers and users are more likely to build.

This reduces our estimated audit backlog by over three million lines, according to cargo vet suggest.

Note that our crates which depend on those engines, such as wasmtime-fuzzing, are not published to crates.io, so if we fall victim to a supply chain attack against dependencies of these crates, the folks who might be impacted are limited.

Although there is value in also auditing code that might be run by people who clone our git repository, in this case I propose that anyone who is concerned about the risks of supply chain attacks against their development systems should be running fuzzers inside a sandbox. After all, it's a fuzzer: it's specifically designed to try to do anything.

I'd like to especially seek comment from folks who've expressed interest in our use of cargo-vet, like @alexcrichton, @bholley, @cfallin, and @tschneidereit. I'm open to being persuaded that we shouldn't make this change, but I can't currently see that we get any value from auditing these particular dependencies.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 26 2024 at 07:54):

jameysharp requested alexcrichton for a review on PR #8488.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 26 2024 at 07:54):

jameysharp requested wasmtime-default-reviewers for a review on PR #8488.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 26 2024 at 07:57):

jameysharp updated PR #8488.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 26 2024 at 11:40):

alexcrichton submitted PR review:

I think this is a reasonable change to make yeah, we already don't vet these dependencies much and reducing our backlog seems reasonable to me

view this post on Zulip Wasmtime GitHub notifications bot (Apr 26 2024 at 17:36):

fitzgen commented on PR #8488:

Makes sense to me too.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 26 2024 at 19:39):

tschneidereit commented on PR #8488:

Same for me, yeah. It'd be good to have this mentioned somewhere in documentation people running fuzzing would be likely to see

view this post on Zulip Wasmtime GitHub notifications bot (Apr 26 2024 at 21:33):

jameysharp commented on PR #8488:

It'd be good to have this mentioned somewhere in documentation people running fuzzing would be likely to see

Good point. I'll add some text to fuzz/README.md and docs/contributing-fuzzing.md to this PR before merging it. I'm not sure we have anywhere else that anyone is likely to notice.

Also, do we have text somewhere that I should update describing our cargo-vet policy?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 30 2024 at 01:19):

jameysharp requested elliottt for a review on PR #8488.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 30 2024 at 01:19):

jameysharp requested wasmtime-fuzz-reviewers for a review on PR #8488.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 30 2024 at 01:19):

jameysharp updated PR #8488.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 30 2024 at 01:29):

jameysharp commented on PR #8488:

I've pushed some new text for fuzz/README.md. @tschneidereit, does this text work for you?

I considered adding text to docs/contributing-fuzzing.md but it's really about developing fuzz targets. For people who just want to run them it points to fuzz/README.md, so I think the notice above suffices.

I also considered adding text to docs/contributing-coding-guidelines.md, where we talk about cargo vet, but couldn't decide if there's anything that would be useful to say there.

Finally, I considered trying to make suggestions about sandboxes people could try, and got as far as verifying that cargo fuzz run works under unshare --map-current-user --net --ipc --mount on Linux (but not with --pid for some reason). But then I remembered that I don't want to be in the business of recommending security tools.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 30 2024 at 03:44):

github-actions[bot] commented on PR #8488:

Subscribe to Label Action

cc @fitzgen

<details>
This issue or pull request has been labeled: "fuzzing"

Thus the following users have been cc'd because of the following labels:

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (May 02 2024 at 17:58):

alexcrichton commented on PR #8488:

I think this is pretty reasonable so I'm gonna go ahead and flag for merge given the consensus here. @tschneidereit if you've got further thoughts though on the docs though we can always have follow-ups too

view this post on Zulip Wasmtime GitHub notifications bot (May 02 2024 at 18:21):

alexcrichton merged PR #8488.

view this post on Zulip Wasmtime GitHub notifications bot (May 03 2024 at 09:34):

tschneidereit commented on PR #8488:

Thank you for doing this, @jameysharp! I agree that these doc changes are good, and also with your reasoning for not making other doc changes :)


Last updated: Oct 23 2024 at 20:03 UTC