Bug#716188: [Mayhem] Bug report on oggvideotools: oggJoin crashes with exit status 139

[Petter Reinholdtsen]

I am able to reproduce this problem.  Note that the stdin input were
not needed to crash the program.  Valgrind report this when the
program crashes (would be nice if future mayhem reports also used
valgrind to get crash info):

==17025== Memcheck, a memory error detector
==17025== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==17025== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==17025== Command: ../../objs/src/oggJoin -2�@ RA �A
==17025== 
OggDecoder::operator>>: no page available, insert a packet first
==17025== Invalid read of size 1
==17025==    at 0x4C2B0B9: bcmp (mc_replace_strmem.c:889)
==17025==    by 0x416A9C: OggBOSExtractorFactory::getStreamType(OggPage&) (in /home/pere/src/debian/xiph/oggvideotools/objs/src/oggJoin)
==17025==    by 0x4166FE: OggBOSExtractorFactory::extractInformation(OggPage&, ExtractorInformation&) (in /home/pere/src/debian/xiph/oggvideotools/objs/src/oggJoin)
==17025==    by 0x410FCF: oggJoinCmd(int, char**) (in /home/pere/src/debian/xiph/oggvideotools/objs/src/oggJoin)
==17025==    by 0x411725: main (in /home/pere/src/debian/xiph/oggvideotools/objs/src/oggJoin)
==17025==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==17025== 
==17025== 
==17025== Process terminating with default action of signal 11 (SIGSEGV)
==17025==  Access not within mapped region at address 0x0
==17025==    at 0x4C2B0B9: bcmp (mc_replace_strmem.c:889)
==17025==    by 0x416A9C: OggBOSExtractorFactory::getStreamType(OggPage&) (in /home/pere/src/debian/xiph/oggvideotools/objs/src/oggJoin)
==17025==    by 0x4166FE: OggBOSExtractorFactory::extractInformation(OggPage&, ExtractorInformation&) (in /home/pere/src/debian/xiph/oggvideotools/objs/src/oggJoin)
==17025==    by 0x410FCF: oggJoinCmd(int, char**) (in /home/pere/src/debian/xiph/oggvideotools/objs/src/oggJoin)
==17025==    by 0x411725: main (in /home/pere/src/debian/xiph/oggvideotools/objs/src/oggJoin)
==17025==  If you believe this happened as a result of a stack
==17025==  overflow in your program's main thread (unlikely but
==17025==  possible), you can try to increase the size of the
==17025==  main thread stack using the --main-stacksize= flag.
==17025==  The main thread stack size used in this run was 8388608.
==17025== 
==17025== HEAP SUMMARY:
==17025==     in use at exit: 76,651 bytes in 16 blocks
==17025==   total heap usage: 19 allocs, 3 frees, 76,718 bytes allocated
==17025== 
==17025== LEAK SUMMARY:
==17025==    definitely lost: 0 bytes in 0 blocks
==17025==    indirectly lost: 0 bytes in 0 blocks
==17025==      possibly lost: 167 bytes in 5 blocks
==17025==    still reachable: 76,484 bytes in 11 blocks
==17025==         suppressed: 0 bytes in 0 blocks
==17025== Rerun with --leak-check=full to see details of leaked memory
==17025== 
==17025== For counts of detected and suppressed errors, rerun with: -v
==17025== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 4 from 4)
./crash.sh: line 21: 17025 Segmentation fault      env -i MALLOC_CHECK_=0 $GDB valgrind ../../objs/src/oggJoin "`cat $DIR/argv_1.symb`" "`cat $DIR/argv_2.symb`" "`cat $DIR/argv_3.symb`"

The following patch fixes the crash:

diff --git a/src/oggBOSExtractorFactory.cpp b/src/oggBOSExtractorFactory.cpp
index 35e07ab..9849acf 100644
--- a/src/oggBOSExtractorFactory.cpp
+++ b/src/oggBOSExtractorFactory.cpp
@@ -116,6 +116,9 @@ OggType OggBOSExtractorFactory::getStreamType(OggPage& page)
 {
   uint8* type = page.obj()->data+page.obj()->headerLength;
 
+  if (NULL == OggTypeMap || NULL == type)
+    return (ogg_unknown);
+
   uint8 i=1;
   for (; i< ogg_maxOggType; ++i) {
     if (memcmp(type, OggTypeMap[i], MAXIDCHARS) == 0)
@@ -131,6 +134,9 @@ OggType OggBOSExtractorFactory::getStreamType(OggPacket& packet)
 {
   uint8* type = packet.data();
 
+  if (NULL == OggTypeMap || NULL == type)
+    return (ogg_unknown);
+
   uint8 i=1;
   for (; i< ogg_maxOggType; ++i) {
     if (memcmp(type, OggTypeMap[i], MAXIDCHARS) == 0)

Only the first block was needed, but the two functions next to each
other were so similar I thought it best to fix it both places.

-- 
Happy hacking
Petter Reinholdtsen