Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve OSS-Fuzz integration #264

Open
3 of 5 tasks
athre0z opened this issue Nov 10, 2021 · 1 comment
Open
3 of 5 tasks

Improve OSS-Fuzz integration #264

athre0z opened this issue Nov 10, 2021 · 1 comment
Assignees
Labels
A-build Area: Build system A-fuzzing Area: Fuzzing tools / OSS-fuzz C-enhancement Category: Enhancement of existing features P-medium Priority: Medium

Comments

@athre0z
Copy link
Member

athre0z commented Nov 10, 2021

Due to the changes in the encoder PR (#254), the build on oss-fuzz is now failing. When fixing it, we should use the opportunity to also:

  • Add the new encoder fuzzing targets
  • Restrict the fuzzer to a sensible input file size to increase fuzzer efficiency (suggested by @mappzor)
  • Add fuzzing corpora for the encoder targets
  • Add our command line tools ./ZydisDisasm and ./ZydisInfo to be fuzzed as well
  • Add fuzzing coverage for the Disassembler.h API
@athre0z athre0z added C-bug Category: This is a bug (or a fix for a bug, when applied to PRs) P-medium Priority: Medium A-build Area: Build system A-fuzzing Area: Fuzzing tools / OSS-fuzz labels Nov 10, 2021
@athre0z athre0z self-assigned this Nov 10, 2021
@mappzor
Copy link
Contributor

mappzor commented Nov 11, 2021

Add fuzzing corpora for the encoder targets

I've checked that structures under clang-cl on Windows and clang on Linux are ABI-compatible (I had some doubts), so we can re-use my existing corpora 😀

Restrict the fuzzer to a sensible input file size to increase fuzzer efficiency (suggested by @mappzor)

Here are the current minimums:

  • Decoder: sizeof(ZydisFuzzControlBlock) (272) + sizeof(buffer) (32) = 304
  • ReEncoding: sizeof(ZydisFuzzControlBlock) (8) + sizeof(buffer) (32) = 40
  • Encoding: sizeof(ZydisEncoderRequest) (392)

I don't think buffer variables need to be that long though. While it's good to have few bytes more than ZYDIS_MAX_INSTRUCTION_LENGTH (15), 20 should be enough. For encoding target it's probably better to use something slightly bigger than size of encoder request, not for fuzzing itself but to make maintenance easier. Using minimum value means that even the smallest addition to structure requires PR to oss-fuzz. Same argument can be made for other fuzz targets but I think encoding is potentially the most affected.

@athre0z athre0z added C-enhancement Category: Enhancement of existing features and removed C-bug Category: This is a bug (or a fix for a bug, when applied to PRs) labels Oct 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build Area: Build system A-fuzzing Area: Fuzzing tools / OSS-fuzz C-enhancement Category: Enhancement of existing features P-medium Priority: Medium
Projects
None yet
Development

No branches or pull requests

2 participants