pts / sam2p

raster (bitmap) image converter with smart PDF and PostScript (EPS) output
http://pts.50.hu/sam2p/
GNU General Public License v2.0
42 stars 15 forks source link

Heap Buffer Overflow-1 in function DGifDecompressLine() in cgif.c #37

Closed Xin-Jiang closed 6 years ago

Xin-Jiang commented 6 years ago

Here is the bug: 1248 else { 1249 / Its a code to needed to be traced: trace the linked list / 1250 / until the prefix is a pixel, while pushing the suffix / 1251 / pixels on our stack. If we done, pop the stack in reverse / 1252 / (thats what stack is good for!) order to output. / 1253 if (Prefix[CrntCode] == NO_SUCH_CODE) { in line 1253 , CrntCode should be checked cause Prefix is a array which has LZ_MAX_CODE (4096) size: unsigned int Prefix[LZ_MAX_CODE+1];

The crash appears as follows: (gdb) run crash000002 1.pdf Program received signal SIGSEGV, Segmentation fault. 0x0000000000412159 in DGifDecompressLine (Line=0x7ffff7f74010 "", LineLen=486109, GifFile=0x691740) at cgif.c:1253 1253 if (Prefix[CrntCode] == NO_SUCH_CODE) { (gdb) bt

0 0x0000000000412159 in DGifDecompressLine (Line=0x7ffff7f74010 "", LineLen=486109, GifFile=0x691740) at cgif.c:1253

1 0x00000000004132eb in CGIF::DGifGetLine (GifFile=0x691740, Line=, LineLen=) at cgif.c:939

2 0x00000000004136ba in CGIF::DGifSlurp (GifFile=GifFile@entry=0x691740) at cgif.c:1508

3 0x000000000041391d in in_gif_reader (ufd=) at in_gif.cpp:48

4 0x000000000042fca8 in Image::load (ufd0=0x66a010, loadHints=..., format=format@entry=0x0) at image.cpp:1428

5 0x0000000000401eb0 in run_sam2p_engine (sout=..., serr=..., argv1=, helpp=helpp@entry=false) at sam2p_main.cpp:1055

6 0x00000000004014d0 in main (argv=0x7fffffffe5c8) at sam2p_main.cpp:1148

(gdb) p CrntCode $1 = 1936269427 (gdb)

sfowl commented 6 years ago

This bug and https://github.com/pts/sam2p/issues/38 apply to cgif.c which is a merge of some GIF-decoding files from giflib. If the bugs are reproducible with giflib, please file bugs against giflib as well:

https://sourceforge.net/p/giflib/bugs/

I have no affiliation with either sam2ps or giflib.

fgeek commented 6 years ago

@Xin-Jiang could you attach the reproducer file to this issue report, thanks.

pts commented 6 years ago

Thank you for reporting this bug!

Line 1253 seems to be fine, because DGifDecompressInput guarantees CrntCode < 4096. Maybe the bug is somewhere else.

Could you please attach the crash000002 file to this issue, so that I can reprodue the crash and find the culprit?

pts commented 6 years ago

Closing this bug now. I'll reopen it as soon as more information is attached.

fgeek commented 6 years ago

@pts Could you send me a email to henri@nerv.fi and I'll send you some samples. Might be easier to debug when you have few of them at hand.

fgeek commented 6 years ago

@pts Attached few samples crashing or causing denial of service situation when build with ASan. Not directly related to this issue report, but might be useful. Command:

sam2p sample EPS: test.eps

2018-07-sam2p-crashes.zip

pts commented 6 years ago

@fgeek: Thanks for the sample inputs in 2018-07-sam2p-crashes.zip . I have fixed some of the bugs (and pushed the fixes to GitHub), but I need more time to diagnose and fix the rest of them. I'll update this issue when done.

fgeek commented 6 years ago

Great! Thank you.

dd8 commented 6 years ago

The code in the sam2p project looks like it has been copied from GifLib 3 which was released in 1996. The Gif library header in sam2p shows:

define GIF_LIB_VERSION " Version 3.0, " https://github.com/pts/sam2p/blob/master/cgif.h

For comparision here's the GifLib 4 library header from 2005: https://sourceforge.net/p/giflib/code/ci/b0414fec70f81257e9a7fb6ed19b5596d7941bb6/tree/lib/gif_lib.h

and the current 5.1.4 version https://sourceforge.net/p/giflib/code/ci/master/tree/lib/gif_lib.h

If sam2p really is using GifLib 3 code then it's missing 22 years worth of GifLib security fixes ...

pts commented 6 years ago

@dd8: Thank you for the code pointer to the newest GifLib.

sam2p is maintained by an unpaid volunteer, and there is no capacity to do all the library upgrades. In this situation GifLib within sam2p will probably be upgraded only when another volunteer contributes such a patch.

fgeek commented 6 years ago

You should put library updates to your todo list as high priority. I can continue fuzzing anyways with your fixes included. Thanks for your work towards better open-source software :)

stefan-cornelius commented 6 years ago

Hi,

I took me a while to figure it out, but I think I've finally managed to create files that reproduce this issue and https://github.com/pts/sam2p/issues/38. One curiosity is that they only crashed for me when using -O2, not -O0. Additionally, when compiled with afl instrumentation, it also doesn't crash.

Anyways, it seems that upstream giflib already has patches for them. I'll attach a patch that backports the relevant patches.

sam2p_CVEs.patch.txt

pts commented 6 years ago

@stefan-cornelius: Thank you for the patch, I've applied it and pushed it.

I'm still waiting for a .gif input file which breaks sam2p (at commit af05f34db7c27fbd1931a4aa898e1226623072d5). If you have one, please attach it here!

pts commented 6 years ago

Please note that sam2p hasn't been fixed yet for all input files in 2018-07-sam2p-crashes.zip . I think there is only more bug to hunt down. I'll keep this issue open until I finish it.

pts commented 6 years ago

@fgeek: I've created a new issue https://github.com/pts/sam2p/issues/46 to discuss crashes caused by 2018-07-sam2p-crashes.zip .

I'm closing this issue. I'll reopen it as soon as I receive an example input file which breaks sam2p (at HEAD or at commit af05f34db7c27fbd1931a4aa898e1226623072d5).

@fgeek, @stefan-cornelius, @dd8, @Xin-Jiang, thank you for all the contributions so far!