Fixed a possible crash with inconsistent SI data

11 files changed, 61 insertions, 50 deletions

diff --git a/CONTRIBUTORS b/CONTRIBUTORS
index ba2661b3..bd3a74a7 100644
--- a/CONTRIBUTORS
+++ b/CONTRIBUTORS
@@ -687,6 +687,7 @@ Marcel Wiesweg <marcel.wiesweg@gmx.de>
  for fixing a memory leak in NIT processing
  for adding a few missing initializations
  for adding play mode pmVideoOnly
+ for fixing a possible crash with inconsistent SI data
 
 Torsten Herz <torsten.herz@web.de>
  for fixing a possible deadlock when using the "Blue" button in the "Schedules" menu
diff --git a/HISTORY b/HISTORY
index b0b71be5..9cd27891 100644
--- a/HISTORY
+++ b/HISTORY
@@ -3006,3 +3006,4 @@ Video Disk Recorder Revision History
   testing regarding buffer performance and giving me some hints that finally led
   to finding out that this was the basic problem causing buffer overflows).
 - Improved Transfer Mode (thanks to Marco Schlüßler for suggestions and testing).
+- Fixed a possible crash with inconsistent SI data (thanks to Marcel Wiesweg).
diff --git a/eit.c b/eit.c
index dac29539..b0cbfdfe 100644
--- a/eit.c
+++ b/eit.c
@@ -8,7 +8,7 @@
  * Robert Schneider <Robert.Schneider@web.de> and Rolf Hakenes <hakenes@hippomi.de>.
  * Adapted to 'libsi' for VDR 1.3.0 by Marcel Wiesweg <marcel.wiesweg@gmx.de>.
  *
- * $Id: eit.c 1.96 2004/07/18 10:52:58 kls Exp $
+ * $Id: eit.c 1.97 2004/10/16 09:49:13 kls Exp $
  */
 
 #include "eit.h"
@@ -47,10 +47,8 @@ cEIT::cEIT(cSchedules *Schedules, int Source, u_char Tid, const u_char *Data)
   bool Modified = false;
 
   SI::EIT::Event SiEitEvent;
-  for (SI::Loop::Iterator it; eventLoop.hasNext(it); ) {
+  for (SI::Loop::Iterator it; eventLoop.getNext(SiEitEvent, it); ) {
       Empty = false;
-      SiEitEvent = eventLoop.getNext(it);
-
       cEvent *pEvent = (cEvent *)pSchedule->GetEvent(SiEitEvent.getEventId(), SiEitEvent.getStartTime());
       if (!pEvent) {
          // If we don't have that event yet, we create a new one.
diff --git a/libsi/descriptor.c b/libsi/descriptor.c
index eb921c95..207b7c25 100644
--- a/libsi/descriptor.c
+++ b/libsi/descriptor.c
@@ -6,7 +6,7 @@
  *   the Free Software Foundation; either version 2 of the License, or     *
  *   (at your option) any later version.                                   *
  *                                                                         *
- *   $Id: descriptor.c 1.13 2004/06/06 14:47:30 kls Exp $
+ *   $Id: descriptor.c 1.14 2004/10/16 09:51:05 kls Exp $
  *                                                                         *
  ***************************************************************************/
 
@@ -110,9 +110,7 @@ char *ExtendedEventDescriptors::getText(char *buffer, int size, const char *sepa
          continue;
 
       ExtendedEventDescriptor::Item item;
-      for (Loop::Iterator it; d->itemLoop.hasNext(it);   ) {
-         item=d->itemLoop.getNext(it);
-
+      for (Loop::Iterator it; d->itemLoop.getNext(item, it);   ) {
          if (!separated && size > sepLen2) {
             strcpy(buffer+index, separation2); // let's have a separator between the long text and the items
             index += sepLen2;
@@ -207,9 +205,7 @@ char *ExtendedEventDescriptors::getTextItemized(char *buffer, int size, const ch
          continue;
 
       ExtendedEventDescriptor::Item item;
-      for (Loop::Iterator it; d->itemLoop.hasNext(it);   ) {
-         item=d->itemLoop.getNext(it);
-
+      for (Loop::Iterator it; d->itemLoop.getNext(item, it);   ) {
          item.itemDescription.getText(buffer+index, size);
          len = strlen(buffer+index);
          index += len;
@@ -251,9 +247,7 @@ bool ExtendedEventDescriptors::getTextItemized(Loop::Iterator &it, bool &valid,
          continue;
 
       ExtendedEventDescriptor::Item item;
-      if (d->itemLoop.hasNext(it)) {
-         item=d->itemLoop.getNext(it);
-
+      if (d->itemLoop.getNext(item, it)) {
          item.item.getText(itemDescription, sizeItemDescription);
          item.itemDescription.getText(itemText, sizeItemText);
          valid=true;
diff --git a/libsi/si.c b/libsi/si.c
index 40f94539..20ca1b7a 100644
--- a/libsi/si.c
+++ b/libsi/si.c
@@ -6,7 +6,7 @@
  *   the Free Software Foundation; either version 2 of the License, or     *
  *   (at your option) any later version.                                   *
  *                                                                         *
- *   $Id: si.c 1.11 2004/06/06 14:43:56 kls Exp $
+ *   $Id: si.c 1.12 2004/10/16 09:54:05 kls Exp $
  *                                                                         *
  ***************************************************************************/
 
@@ -30,6 +30,10 @@ void Object::setData(CharArray &d) {
    data=d;
 }
 
+bool Object::checkSize(unsigned int offset) {
+   return data.checkSize(offset);
+}
+
 Section::Section(const unsigned char *data, bool doCopy) {
    setData(data, getLength(data), doCopy);
 }
@@ -50,15 +54,15 @@ int Section::getLength(const unsigned char *d) {
    return HILO(((const SectionHeader *)d)->section_length)+sizeof(SectionHeader);
 }
 
-bool CRCSection::isValid() {
+bool CRCSection::isCRCValid() {
    return CRC32::isValid((const char *)data.getData(), getLength()/*, data.FourBytes(getLength()-4)*/);
 }
 
 bool CRCSection::CheckCRCAndParse() {
-   if (!isValid())
+   if (!isCRCValid())
       return false;
    CheckParse();
-   return true;
+   return isValid();
 }
 
 int NumberedSection::getTableIdExtension() const {
@@ -102,7 +106,7 @@ DescriptorTag Descriptor::getDescriptorTag(const unsigned char *d) {
 }
 
 Descriptor *DescriptorLoop::getNext(Iterator &it) {
-   if (it.i<getLength()) {
+   if (isValid() && it.i<getLength()) {
       return createDescriptor(it.i, true);
    }
    return 0;
@@ -110,7 +114,7 @@ Descriptor *DescriptorLoop::getNext(Iterator &it) {
 
 Descriptor *DescriptorLoop::getNext(Iterator &it, DescriptorTag tag, bool returnUnimplemetedDescriptor) {
    Descriptor *d=0;
-   if (it.i<getLength()) {
+   if (isValid() && it.i<getLength()) {
       const unsigned char *p=data.getData(it.i);
       const unsigned char *end=p+getLength()-it.i;
       while (p < end) {
@@ -128,7 +132,7 @@ Descriptor *DescriptorLoop::getNext(Iterator &it, DescriptorTag tag, bool return
 
 Descriptor *DescriptorLoop::getNext(Iterator &it, DescriptorTag *tags, int arrayLength, bool returnUnimplementedDescriptor) {
    Descriptor *d=0;
-   if (it.i<getLength()) {
+   if (isValid() && it.i<getLength()) {
       const unsigned char *p=data.getData(it.i);
       const unsigned char *end=p+getLength()-it.i;
       while (p < end) {
@@ -147,6 +151,8 @@ Descriptor *DescriptorLoop::getNext(Iterator &it, DescriptorTag *tags, int array
 }
 
 Descriptor *DescriptorLoop::createDescriptor(int &i, bool returnUnimplemetedDescriptor) {
+   if (!checkSize(Descriptor::getLength(data.getData(i))))
+      return 0;
    Descriptor *d=Descriptor::getDescriptor(data+i, domain, returnUnimplemetedDescriptor);
    if (!d)
       return 0;
diff --git a/libsi/si.h b/libsi/si.h
index 85d16ed4..8f89c343 100644
--- a/libsi/si.h
+++ b/libsi/si.h
@@ -6,7 +6,7 @@
  *   the Free Software Foundation; either version 2 of the License, or     *
  *   (at your option) any later version.                                   *
  *                                                                         *
- *   $Id: si.h 1.10 2004/06/06 13:35:21 kls Exp $
+ *   $Id: si.h 1.11 2004/10/16 09:58:10 kls Exp $
  *                                                                         *
  ***************************************************************************/
 
@@ -180,12 +180,17 @@ public:
    //can only be called once since data is immutable
    void setData(const unsigned char*data, unsigned int size, bool doCopy=true);
    CharArray getData() { return data; }
+   //returns the valid flag which indicates if data is all right or errors have been encountered
+   bool isValid() { return data.isValid(); }
    virtual int getLength() = 0;
 protected:
    CharArray data;
    //is protected - not used for sections
    template <class T> friend class StructureLoop;
    void setData(CharArray &d);
+   //returns whether the given offset fits within the limits of the actual data
+   //The valid flag will be set accordingly
+   bool checkSize(unsigned int offset);
 };
 
 class Section : public Object {
@@ -205,7 +210,7 @@ public:
    //convenience: sets data and parses if doParse
    CRCSection(const unsigned char *data, bool doCopy=true) : Section(data, doCopy) {}
    CRCSection() {}
-   bool isValid();
+   bool isCRCValid();
    //convenience: isValid+CheckParse
    bool CheckCRCAndParse();
 };
@@ -229,9 +234,9 @@ public:
 class VariableLengthPart : public Object {
 public:
    //never forget to call this
-   void setData(CharArray d, int l) { Object::setData(d); length=l; }
+   void setData(CharArray d, int l) { Object::setData(d); checkSize(l); length=l; }
    //convenience method
-   void setDataAndOffset(CharArray d, int l, unsigned int &offset) { Object::setData(d); length=l; offset+=l; }
+   void setDataAndOffset(CharArray d, int l, unsigned int &offset) { Object::setData(d); checkSize(l); length=l; offset+=l; }
    virtual int getLength() { return length; }
 private:
    int length;
@@ -281,29 +286,36 @@ template <class T> class StructureLoop : public Loop {
 public:
    //currently you must use a while-loop testing for hasNext()
    //i must be 0 to get the first descriptor (with the first call)
-   T getNext(Iterator &it)
+   bool getNext(T &obj, Iterator &it)
       {
+         if (!isValid() || it.i >= getLength())
+            return false;
          CharArray d=data;
          d.addOffset(it.i);
          T ret;
          ret.setData(d);
          ret.CheckParse();
+         if (!checkSize(ret.getLength()))
+            return false;
          it.i+=ret.getLength();
-         return ret;
+         obj=ret;
+         return true;
       }
    T* getNextAsPointer(Iterator &it)
       {
-         if (getLength() <= it.i)
+         if (!isValid() || it.i >= getLength())
             return 0;
          CharArray d=data;
          d.addOffset(it.i);
          T *ret=new T();
          ret->setData(d);
          ret->CheckParse();
+         if (!checkSize(ret->getLength()))
+            return 0;
          it.i+=ret->getLength();
          return ret;
       }
-   bool hasNext(Iterator &it) { return getLength() > it.i; }
+   //bool hasNext(Iterator &it) { return getLength() > it.i; }
 };
 
 //contains descriptors of different types
@@ -385,7 +397,7 @@ public:
          it.i+=sizeof(T);
          return ret;
       }
-   bool hasNext(Iterator &it) { return getLength() > it.i; }
+   bool hasNext(Iterator &it) { return isValid() && (getLength() > it.i); }
 };
 
 class MHP_DescriptorLoop : public DescriptorLoop {
diff --git a/libsi/util.c b/libsi/util.c
index a2a6f0d2..c6ad234a 100644
--- a/libsi/util.c
+++ b/libsi/util.c
@@ -6,7 +6,7 @@
  *   the Free Software Foundation; either version 2 of the License, or     *
  *   (at your option) any later version.                                   *
  *                                                                         *
- *   $Id: util.c 1.3 2003/12/22 14:03:03 kls Exp $
+ *   $Id: util.c 1.4 2004/10/16 09:58:41 kls Exp $
  *                                                                         *
  ***************************************************************************/
 
@@ -88,9 +88,7 @@ CharArray CharArray::operator+(const unsigned int offset) const {
    return f;
 }
 
-CharArray::Data::Data() : count_(1) {
-   size=0;
-   data=0;
+CharArray::Data::Data() : data(0), size(0), count_(1), valid(true) {
    /*
    lockingPid = 0;
    locked = 0;
diff --git a/libsi/util.h b/libsi/util.h
index db019238..db2a0a17 100644
--- a/libsi/util.h
+++ b/libsi/util.h
@@ -6,7 +6,7 @@
  *   the Free Software Foundation; either version 2 of the License, or     *
  *   (at your option) any later version.                                   *
  *                                                                         *
- *   $Id: util.h 1.3 2003/12/22 14:07:41 kls Exp $
+ *   $Id: util.h 1.4 2004/10/16 09:59:48 kls Exp $
  *                                                                         *
  ***************************************************************************/
 
@@ -58,6 +58,9 @@ public:
    u_int16_t TwoBytes(const unsigned int index) const { return data_->data ? data_->TwoBytes(off+index) : 0; }
    u_int32_t FourBytes(const unsigned int index) const { return data_->data ? data_->FourBytes(off+index) : 0; }
 
+   bool isValid() const { return data_->valid; }
+   bool checkSize(unsigned int offset) { return (data_->valid && (data_->valid=(off+offset < data_->size))); }
+
    void addOffset(unsigned int offset) { off+=offset; }
 private:
    class Data {
@@ -86,10 +89,12 @@ private:
       const unsigned char*data;
       unsigned int size;
 
-      unsigned count_;
       // count_ is the number of CharArray objects that point at this
       // count_ must be initialized to 1 by all constructors
       // (it starts as 1 since it is pointed to by the CharArray object that created it)
+      unsigned count_;
+
+      bool valid;
 
       /*
       pthread_mutex_t mutex;
diff --git a/nit.c b/nit.c
index c515c240..494d561d 100644
--- a/nit.c
+++ b/nit.c
@@ -4,7 +4,7 @@
  * See the main source file 'vdr.c' for copyright information and
  * how to reach the author.
  *
- * $Id: nit.c 1.8 2004/06/06 14:24:49 kls Exp $
+ * $Id: nit.c 1.9 2004/10/16 10:00:27 kls Exp $
  */
 
 #include "nit.h"
@@ -92,8 +92,8 @@ void cNitFilter::Process(u_short Pid, u_char Tid, const u_char *Data, int Length
      return;
   if (!Channels.Lock(true, 10))
      return;
-  for (SI::Loop::Iterator it; nit.transportStreamLoop.hasNext(it); ) {
-      SI::NIT::TransportStream ts = nit.transportStreamLoop.getNext(it);
+  SI::NIT::TransportStream ts;
+  for (SI::Loop::Iterator it; nit.transportStreamLoop.getNext(ts, it); ) {
       SI::Descriptor *d;
       for (SI::Loop::Iterator it2; (d = ts.transportStreamDescriptors.getNext(it2)); ) {
           switch (d->getDescriptorTag()) {
diff --git a/pat.c b/pat.c
index 260f67d3..a0124f7b 100644
--- a/pat.c
+++ b/pat.c
@@ -4,7 +4,7 @@
  * See the main source file 'vdr.c' for copyright information and
  * how to reach the author.
  *
- * $Id: pat.c 1.9 2004/05/23 09:29:04 kls Exp $
+ * $Id: pat.c 1.10 2004/10/16 10:01:12 kls Exp $
  */
 
 #include "pat.h"
@@ -285,8 +285,7 @@ void cPatFilter::Process(u_short Pid, u_char Tid, const u_char *Data, int Length
               return;
            SI::PAT::Association assoc;
            int Index = 0;
-           for (SI::Loop::Iterator it; pat.associationLoop.hasNext(it); ) {
-               assoc = pat.associationLoop.getNext(it);
+           for (SI::Loop::Iterator it; pat.associationLoop.getNext(assoc, it); ) {
                if (!assoc.isNITPid()) {
                   if (Index++ == pmtIndex) {
                      pmtPid = assoc.getPid();
@@ -332,8 +331,7 @@ void cPatFilter::Process(u_short Pid, u_char Tid, const u_char *Data, int Length
         int Tpid = 0;
         int NumApids = 0;
         int NumDpids = 0;
-        for (SI::Loop::Iterator it; pmt.streamLoop.hasNext(it); ) {
-            stream = pmt.streamLoop.getNext(it);
+        for (SI::Loop::Iterator it; pmt.streamLoop.getNext(stream, it); ) {
             switch (stream.getStreamType()) {
               case 1: // STREAMTYPE_11172_VIDEO
               case 2: // STREAMTYPE_13818_VIDEO
diff --git a/sdt.c b/sdt.c
index efcad3d5..92d5a740 100644
--- a/sdt.c
+++ b/sdt.c
@@ -4,7 +4,7 @@
  * See the main source file 'vdr.c' for copyright information and
  * how to reach the author.
  *
- * $Id: sdt.c 1.11 2004/07/18 11:14:42 kls Exp $
+ * $Id: sdt.c 1.12 2004/10/16 10:02:23 kls Exp $
  */
 
 #include "sdt.h"
@@ -39,9 +39,7 @@ void cSdtFilter::Process(u_short Pid, u_char Tid, const u_char *Data, int Length
   if (!Channels.Lock(true, 10))
      return;
   SI::SDT::Service SiSdtService;
-  for (SI::Loop::Iterator it; sdt.serviceLoop.hasNext(it); ) {
-      SiSdtService = sdt.serviceLoop.getNext(it);
-
+  for (SI::Loop::Iterator it; sdt.serviceLoop.getNext(SiSdtService, it); ) {
       cChannel *channel = Channels.GetByChannelID(tChannelID(Source(), sdt.getOriginalNetworkId(), sdt.getTransportStreamId(), SiSdtService.getServiceId()));
       if (!channel)
          channel = Channels.GetByChannelID(tChannelID(Source(), 0, Transponder(), SiSdtService.getServiceId()));
@@ -110,8 +108,8 @@ void cSdtFilter::Process(u_short Pid, u_char Tid, const u_char *Data, int Length
             */
             case SI::NVODReferenceDescriptorTag: {
                  SI::NVODReferenceDescriptor *nrd = (SI::NVODReferenceDescriptor *)d;
-                 for (SI::Loop::Iterator it; nrd->serviceLoop.hasNext(it); ) {
-                     SI::NVODReferenceDescriptor::Service Service = nrd->serviceLoop.getNext(it);
+                 SI::NVODReferenceDescriptor::Service Service;
+                 for (SI::Loop::Iterator it; nrd->serviceLoop.getNext(Service, it); ) {
                      cChannel *link = Channels.GetByChannelID(tChannelID(Source(), Service.getOriginalNetworkId(), Service.getTransportStream(), Service.getServiceId()));
                      if (!link && Setup.UpdateChannels >= 3) {
                         link = Channels.NewChannel(Channel(), "NVOD", Service.getOriginalNetworkId(), Service.getTransportStream(), Service.getServiceId());