1447204 - Vorbis fuzzing target review

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, enhancement, P3)

Description

[Tyson Smith [:tsmith]]

Attachment: vorbis_coverage.zip

Here is a summary of what is currently covered by our AFL fuzzing efforts targeting examples/decoder_example. The full report generated with LCOV is in the attached zip. This was generated by running a copy of the current corpus.

Filename        Line Coverage          Functions
bitrate.c        2.4 %     3 / 124     20.0 %    1 / 5
block.c         56.1 %   264 / 471     61.1 %   11 / 18
codebook.c      71.4 %   132 / 185     80.0 %    8 / 10
envelope.c       0.0 %     0 / 169      0.0 %    0 / 6
floor0.c         100 %    94 / 94       100 %    7 / 7
floor1.c        34.8 %   190 / 546     47.4 %    9 / 19
info.c          42.6 %   152 / 357     37.5 %    9 / 24
lpc.c            0.0 %     0 / 39       0.0 %    0 / 2
lsp.c           20.8 %    21 / 101     16.7 %    1 / 6
mapping0.c      44.1 %   101 / 229     60.0 %    3 / 5
mdct.c          86.4 %   306 / 354     90.9 %   10 / 11
psy.c            0.0 %     0 / 571      0.0 %    0 / 23
res0.c          48.7 %   169 / 347     47.4 %    9 / 19
scales.h         0.0 %     0 / 4        0.0 %    0 / 0
sharedbook.c    82.5 %   170 / 206     71.4 %   10 / 14
smallft.c        0.6 %     5 / 816      6.7 %    1 / 15
synthesis.c     41.7 %    30 / 72      20.0 %    1 / 5
window.c         8.7 %     2 / 23      50.0 %    1 / 2

Does this cover everything we would like to have covered (everything the browser uses)? If not should we create a different target for fuzzing such as the one found in oss-fuzz[1]?

[1] https://github.com/google/oss-fuzz/blob/master/projects/vorbis/decode_fuzzer.cc

Comment 1

[Thomas Daede [:TD-Linux]]

oss-fuzz also only tests decoding, so it should be covering about the same amount of coverage as our fuzzer - does it have something else that we do not?

Comment 2

[Tom Ritter [:tjr]]

Is this still something we need to do? If not, can we close it?

Comment 3

[Tyson Smith [:tsmith]]

Someone who knows the code base better than me might have some insight. If not I think it is safe to close this bug.
Here is the latest code coverage information from oss-fuzz:

Path	Line Coverage	Function Coverage	Region Coverage
bitrate.c	1.82% (4/220)	20.00% (1/5)	0.83% (1/121)
block.c	53.31% (491/921)	61.11% (11/18)	53.41% (243/455)
codebook.c	69.05% (270/391)	80.00% (8/10)	74.38% (180/242)
envelope.c	0.00% (0/281)	0.00% (0/6)	0.00% (0/156)
floor0.c	100.00% (151/151)	100.00% (7/7)	98.78% (81/82)
floor1.c	36.17% (327/904)	47.37% (9/19)	40.22% (179/445)
info.c	46.53% (282/606)	40.00% (10/25)	61.06% (265/434)
lpc.c	0.00% (0/98)	0.00% (0/2)	0.00% (0/43)
lsp.c	17.74% (33/186)	16.67% (1/6)	14.02% (15/107)
mapping0.c	31.32% (166/530)	60.00% (3/5)	53.33% (112/210)
mdct.c	85.48% (418/489)	90.91% (10/11)	84.86% (157/185)
os.h	100.00% (9/9)	100.00% (3/3)	100.00% (3/3)
psy.c	0.00% (0/983)	0.00% (0/23)	0.00% (0/631)
res0.c	48.17% (290/602)	47.37% (9/19)	52.80% (198/375)
scales.h	0.00% (0/18)	0.00% (0/2)	0.00% (0/2)
sharedbook.c	86.60% (349/403)	71.43% (10/14)	88.46% (230/260)
smallft.c	0.58% (7/1199)	6.67% (1/15)	1.01% (5/495)
synthesis.c	42.28% (63/149)	40.00% (2/5)	38.46% (35/91)
vorbisfile.c	27.48% (567/2063)	27.87% (17/61)	25.58% (344/1345)
window.c	8.33% (3/36)	50.00% (1/2)	5.56% (1/18)

Comment 4

[Bryce Seager van Dyk [:bryce] (he/him) - Not reading bugmail]

Monty, do you have any suggestions as to if we want more coverage on any of these files?

Comment 5

[Monty Montgomery (:xiphmont)]

I need to look at it more carefully than a once-over. That's approximately the kind of coverage I'd expect from decode-only, but I want to make sure nothing obvious is missing.

Comment 6

[Tom Ritter [:tjr]]

Hey Monty, wanted to ask if had found time to dig into this more, or felt it was acceptable.

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

Clear a needinfo that is pending on an inactive user.

Inactive users most likely will not respond; if the missing information is essential and cannot be collected another way, the bug maybe should be closed as INCOMPLETE.

For more information, please visit BugBot documentation.

Comment 8

[Tom Ritter [:tjr]]

Do we want a second opinion? Maybe pass the ni to someone from the media team?

Comment 9

[Tyson Smith [:tsmith]]

Since this is now sandboxed via rlbox this can be unhidden.

The latest coverage data is public and can be found here: https://oss-fuzz.com/coverage-report/job/libfuzzer_asan_vorbis/latest

Jim: Is there anyone on the media team that would be interested in this data?

Comment 10

[Tom Ritter [:tjr]]

It is? I'm probably confused about how these libraries interact - but libogg is rlboxed, and while there are references to vorbis there, the tainted things I'm seeing (like ogg_page) are in libogg...

Comment 11

[Jim Mathies [:jimm]]

Paul, any opinion here? Media team wasn't sure if we consider this good coverage. I'm also going to assume other companies cover fizzing this as well.

Comment 12

[Paul Adenot (:padenot)]

Vorbis isn't in rlbox. Vorbis is a codec, OGG is a container (that can contain Vorbis, but also other things such as Opus or Flac). OGG has been rlboxed.

Looking through a few files, this looks what I'd expect for decode side. We don't ship and will not ship a Vorbis encoder at this point.