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

Fix broken fuzz targets #63

Merged
merged 4 commits into from
Jul 5, 2024
Merged

Fix broken fuzz targets #63

merged 4 commits into from
Jul 5, 2024

Conversation

CUB3D
Copy link
Collaborator

@CUB3D CUB3D commented Jul 5, 2024

Fixes the following fuzz targets:

  • amf0_header
  • amf0_element_array
  • amf3_string
  • amf3_int_unsigned
  • amf3_int_signed

Removes the following trivial fuzz targets:

  • amf0_length
  • amf0_string
  • amf0_signature
  • amf0_version
  • amf0_bool
  • amf0_number
  • amf0_string
  • amf0_long_string

All fuzz cases now build and run, however amf3_body still fails due to issues with Rc

Gates amf3 behind feature "amf3" so the amf0 fuzzer doesn't start wandering off, default is enabled

Updates dev/fuzzing deps
Fixes workspace resolver warning

@Dinnerbone
Copy link
Contributor

Dinnerbone commented Jul 5, 2024

Gates amf3 behind feature "amf3" so the amf0 fuzzer doesn't start wandering off, default is enabled

That's kind of a problem with our structure atm; we need to be able to tell something to stick to amf0.

In actual spec, amf3 serialization writes amf0 by default and only elevates to amf3 as-required. amf0 serializers shouldn't be able to upgrade to amf3

@CUB3D
Copy link
Collaborator Author

CUB3D commented Jul 5, 2024

I think these are two different problems, this is trying to fix the issue of TypeMarker::AMF3 tags sending the amf0 fuzzer off down the amf3 path when decoding, which should be getting coverage from the amf3 fuzzer anyway.
I do remember having to make a lot of guesses about the serialization side of things, evidently quite a few were wrong

@Dinnerbone Dinnerbone merged commit 99b16db into master Jul 5, 2024
1 check passed
@torokati44 torokati44 mentioned this pull request Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants