viniul / oss-fuzz

OSS-Fuzz - continuous fuzzing of open source software
Apache License 2.0
0 stars 0 forks source link

Compare coverage data of proto and nonproto fuzzer #7

Open bshastry opened 5 years ago

bshastry commented 5 years ago

This requires qualitative analysis of coverage data.

Which source lines of code are not covered by proto in comparison to nonproto fuzzer? Why?

What can we do to improve proto coverage?

bshastry commented 5 years ago

What are the following functions not covered? What can we do to cover them?

and these functions in gifalloc.c

Why is the following code segment uncovered? What can we do to cover it?

1187 | 8 | if (GifFile->ExtensionBlocks) {
-- | -- | --
1188 | 0 | sp->ExtensionBlocks = GifFile->ExtensionBlocks;
1189 | 0 | sp->ExtensionBlockCount = GifFile->ExtensionBlockCount;
1190 | 0 |  
1191 | 0 | GifFile->ExtensionBlocks = NULL;
1192 | 0 | GifFile->ExtensionBlockCount = 0;
1193 | 0 | }
viniul commented 5 years ago

Regarding file opening: Current non proto fuzzer also misses file opening functions, since it passes the data directly to gif now.

bshastry commented 5 years ago

I could spot the following differences (lines covered by nonproto by not covered by proto)

211 | 143 | if (strncmp(GIF_STAMP, Buf, GIF_VERSION_POS) != 0) {
-- | -- | --
212 | 1 | if (Error != NULL)
213 | 1 | *Error = D_GIF_ERR_NOT_GIF_FILE;
214 | 1 | free((char *)Private);
215 | 1 | free((char *)GifFile);
216 | 1 | return NULL;
217 | 1 | }

The reason this is not covered by proto is that we always write the correct header.

if (InternalRead(GifFile, Buf, 3) != 3) {
--
259 | 1 | GifFile->Error = D_GIF_ERR_READ_FAILED;
260 | 1 | GifFreeMapObject(GifFile->SColorMap);
261 | 1 | GifFile->SColorMap = NULL;
262 | 1 | return GIF_ERROR;
263 | 1 | }

The reason we don't cover this is because we always have sufficient header data.


/* Setup the colormap */
--
384 | 288 | if (GifFile->Image.ColorMap) {
385 | 6 | GifFreeMapObject(GifFile->Image.ColorMap);
386 | 6 | GifFile->Image.ColorMap = NULL;
387 | 6 | }

Not sure why we don't cover this.

575 | 331 | if (InternalRead(GifFile, &Buf, 1) != 1) {
-- | -- | --
576 | 1 | GifFile->Error = D_GIF_ERR_READ_FAILED;
577 | 1 | return GIF_ERROR;
578 | 1 | }

I think this has to do with insufficient extension data.

1187 | 8 | if (GifFile->ExtensionBlocks) {
-- | -- | --
1188 | 0 | sp->ExtensionBlocks = GifFile->ExtensionBlocks;
1189 | 0 | sp->ExtensionBlockCount = GifFile->ExtensionBlockCount;
1190 | 0 |  
1191 | 0 | GifFile->ExtensionBlocks = NULL;
1192 | 0 | GifFile->ExtensionBlockCount = 0;
1193 | 0 | }

Dunno why not.

bshastry commented 5 years ago

After the m_hasLCT reset (bug fix), we now cover

383 | 226 | /* Setup the colormap */
-- | -- | --
384 | 226 | if (GifFile->Image.ColorMap) {
385 | 6 | GifFreeMapObject(GifFile->Image.ColorMap);
386 | 6 | GifFile->Image.ColorMap = NULL;
387 | 6 | }
bshastry commented 5 years ago

And we now also cover

81 | if (GifFile->ExtensionBlocks) {
-- | --
1188 | 11 | sp->ExtensionBlocks = GifFile->ExtensionBlocks;
1189 | 11 | sp->ExtensionBlockCount = GifFile->ExtensionBlockCount;
1190 | 11 |  
1191 | 11 | GifFile->ExtensionBlocks = NULL;
1192 | 11 | GifFile->ExtensionBlockCount = 0;
1193 | 11 | }
bshastry commented 5 years ago

Why are the following source files in libgif.a not covered

bshastry commented 5 years ago

Easy to add functions from gif_font.c for the harness