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

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

Closed
Xin-Jiang opened this issue May 24, 2018 · 15 comments
Closed

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

Xin-Jiang opened this issue May 24, 2018 · 15 comments
Labels
more info needed to reproduce This issue is blocked on more information from the reporter or from contributors.

Comments

@Xin-Jiang
Copy link

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
Copy link

sfowl commented May 28, 2018

This bug and #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
Copy link

fgeek commented Jul 13, 2018

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

@pts
Copy link
Owner

pts commented Jul 17, 2018

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 pts added the more info needed to reproduce This issue is blocked on more information from the reporter or from contributors. label Jul 17, 2018
@pts
Copy link
Owner

pts commented Jul 25, 2018

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

@pts pts closed this as completed Jul 25, 2018
@fgeek
Copy link

fgeek commented Jul 25, 2018

@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
Copy link

fgeek commented Jul 26, 2018

@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 pushed a commit that referenced this issue Jul 26, 2018
…:separate for 2018-07-sam2p-crashes/sample-100 in #37
@pts pts reopened this Jul 26, 2018
@pts
Copy link
Owner

pts commented Jul 26, 2018

@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
Copy link

fgeek commented Jul 26, 2018

Great! Thank you.

@dd8
Copy link

dd8 commented Jul 29, 2018

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
Copy link
Owner

pts commented Jul 30, 2018

@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
Copy link

fgeek commented Jul 30, 2018

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
Copy link

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 #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
Copy link
Owner

pts commented Aug 6, 2018

@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 af05f34). If you have one, please attach it here!

@pts
Copy link
Owner

pts commented Aug 6, 2018

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
Copy link
Owner

pts commented Sep 17, 2018

@fgeek: I've created a new issue #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 af05f34).

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

@pts pts closed this as completed Sep 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
more info needed to reproduce This issue is blocked on more information from the reporter or from contributors.
Projects
None yet
Development

No branches or pull requests

6 participants