Hi all, I'm working on fuzzing Opus with OSS-Fuzz and have started with the decoder. Attached is a patch to add the corresponding fuzz target. Please let me know if there are any concerns? Thanks, Felicia -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.xiph.org/pipermail/opus/attachments/20170127/f69951aa/attachment-0001.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0001-Add-decoder-fuzz-target.patch Type: text/x-patch Size: 5042 bytes Desc: not available URL: <http://lists.xiph.org/pipermail/opus/attachments/20170127/f69951aa/attachment-0001.bin>
On 2017-01-27 11:48 AM, Felicia Lim wrote:> I'm working on fuzzing Opus with OSS-Fuzz and have started with the > decoder. Attached is a patch to add the corresponding fuzz target.Thanks for writing the fuzzer harness! Some comments below.> + * Copyright (c) 2017 Google Inc. > + * Redistribution and use in source and binary forms, with or without > + [...]Please format this to look like the other copyright headers. I think maybe your editor's auto-indent mangled the previous layout. In ParseToc everything here looks correct.> + info->frame_size = frame_sizes_ms_x2[row][col] * info->fs / 2000;This rounds correctly because everything in as even multple of 1000. In LLVMFuzzerTestOneInput:> + decoder = opus_decoder_create(toc.fs, toc.channels, &err);I guess the fuzzer will vary the packet's declared sample rate and channel count independently of the data, so taking these values from the input data allows exploration of all valid values for the setup without loss of coverage?> + fec = data[1] & 1;As far as I can tell this isn't an independent mechanism, since this bit will be interpreted as packet data half the time and as a byte count for the second packet in others. It seems like this will limit coverage. Is there another channel you can pass data through? I also expect only running one packet at a time through the decoder will limit converage. This is fine for a place to start, but the codec explicitly accumulates state over a window of about 80 ms. With only one packet per run the fuzzer won't be able to explore mode switching, or FEC or PLC on top of more than one initial state. Cheers, -r
Hi Felicia, Here's a few comments/questions on your patch:> static void ParseToc(const uint8_t toc, TocInfo *const info) {Any particular reason you don't use the Opus functions for parsing the ToC? It seems like opus_packet_get_nb_samples(), opus_packet_get_bandwidth(), and opus_packet_get_nb_channels() should do the trick.> int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size)How is that function supposed to work? I noticed that it will only decode a single packet. How does the fuzzer handle testing for bugs that only happen for a given sequence of input packets? Cheers, Jean-Marc On 27/01/17 02:48 PM, Felicia Lim wrote:> Hi all, > > I'm working on fuzzing Opus with OSS-Fuzz and have started with the > decoder. Attached is a patch to add the corresponding fuzz target. > Please let me know if there are any concerns? > > Thanks, > Felicia > > > _______________________________________________ > opus mailing list > opus at xiph.org > http://lists.xiph.org/mailman/listinfo/opus >
Thanks for your comments, Ralph and Jean-Marc. Please find attached the amended patch: - decodes a sequence of input packets rather than just one (I'm planning on using the Opus test vectors as the seed corpus) - decides on decoder setup and FEC independently of the packet data - uses Opus functions to parse ToC Cheers, Felicia On Sun, Jan 29, 2017 at 9:48 PM Jean-Marc Valin <jmvalin at jmvalin.ca> wrote:> Hi Felicia, > > Here's a few comments/questions on your patch: > > > static void ParseToc(const uint8_t toc, TocInfo *const info) { > > Any particular reason you don't use the Opus functions for parsing the > ToC? It seems like opus_packet_get_nb_samples(), > opus_packet_get_bandwidth(), and opus_packet_get_nb_channels() should do > the trick. > > > int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) > > How is that function supposed to work? I noticed that it will only > decode a single packet. How does the fuzzer handle testing for bugs that > only happen for a given sequence of input packets? > > Cheers, > > Jean-Marc > > > > > On 27/01/17 02:48 PM, Felicia Lim wrote: > > Hi all, > > > > I'm working on fuzzing Opus with OSS-Fuzz and have started with the > > decoder. Attached is a patch to add the corresponding fuzz target. > > Please let me know if there are any concerns? > > > > Thanks, > > Felicia > > > > > > _______________________________________________ > > opus mailing list > > opus at xiph.org > > http://lists.xiph.org/mailman/listinfo/opus > > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.xiph.org/pipermail/opus/attachments/20170210/8570012c/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0001-Add-decoder-fuzz-target.patch Type: text/x-patch Size: 5036 bytes Desc: not available URL: <http://lists.xiph.org/pipermail/opus/attachments/20170210/8570012c/attachment.bin>