gpertea / gclib

GCLib - Genomic C++ library of reusable code for bioinformatics projects
Other
33 stars 13 forks source link

Uninitialized GFF line info causes out-of-bounds-read, possible out-of-bounds-write. #11

Open carter-yagemann opened 3 years ago

carter-yagemann commented 3 years ago

Reproduce

PoC Input: min.gz

Steps to Reproduce:

  1. Compile GffRead v0.12.7 (will fetch GCLib v0.12.7)
  2. Decompress PoC input: gzip -d min.gz
  3. Run: ./gffread -E min -o out

Output:

Command line was:
./gffread -E min -o out
Segmentation fault

Root Cause

https://github.com/gpertea/gclib/blob/8aee376774ccb2f3bd3f8e3bf1c9df1528ac7c5b/gff.cpp#L413-L432

When GCLib reads a GFF line with no info segment, the char * at t[8] will not be set, causing it to take on whatever stale value happens to be in that location of the stack. Triggered accidentally, this can cause a segfault due to reading an invalid address here:

https://github.com/gpertea/gclib/blob/8aee376774ccb2f3bd3f8e3bf1c9df1528ac7c5b/gff.cpp#L118

However, a maliciously crafted input may be able to place a valid pointer at this location, causing a more severe vulnerability.

Proposed Patch

At a minimum, t should be zeroed during initialization:

*** v0.12.7/gclib/gff.cpp     2021-07-23 10:31:39.000000000 -0400
--- new/gclib/gff.cpp       2021-10-04 10:54:52.989309121 -0400
*************** GffLine::GffLine(GffReader* reader, cons
*** 405,411 ****
   GMALLOC(dupline, llen+1);
   memcpy(dupline, l, llen+1);
   skipLine=true; //clear only if we make it to the end of this function
!  char* t[9];
   int i=0;
   int tidx=1;
   t[0]=line;
--- 405,411 ----
   GMALLOC(dupline, llen+1);
   memcpy(dupline, l, llen+1);
   skipLine=true; //clear only if we make it to the end of this function
!  char* t[9] = {0};
   int i=0;
   int tidx=1;
   t[0]=line;

Ideally, the library should gracefully handle no info being found (this only works if t is zero initialized):

*** v0.12.7/gclib/gff.cpp     2021-07-23 10:31:39.000000000 -0400
--- new/gclib/gff.cpp       2021-10-04 10:54:52.989309121 -0400
*************** GffLine::GffLine(GffReader* reader, cons
*** 430,435 ****
--- 430,439 ----
   track=t[1];
   ftype=t[2];
   info=t[8];
+  if (!info) {
+    GMessage("Warning: missing info:\n%s\n",l);
+    return;
+  }
   char* p=t[3];
   if (!parseUInt(p,fstart)) {
     //chromosome_band entries in Flybase

Credit

This bug was detected using AFL and localized using ARCUS.

carnil commented 3 years ago

This issue seems to have CVE-2021-42006 assigned.