From ac13b6e161eb7dac3ff3fb14c50d2bec4187f2d1 Mon Sep 17 00:00:00 2001 From: Klaus Schmidinger Date: Sat, 16 Oct 2004 10:14:19 +0200 Subject: [PATCH] Fixed a possible crash with inconsistent SI data --- CONTRIBUTORS | 1 + HISTORY | 1 + eit.c | 6 ++---- libsi/descriptor.c | 14 ++++---------- libsi/si.c | 20 +++++++++++++------- libsi/si.h | 30 +++++++++++++++++++++--------- libsi/util.c | 6 ++---- libsi/util.h | 9 +++++++-- nit.c | 6 +++--- pat.c | 8 +++----- sdt.c | 10 ++++------ 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 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 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 and Rolf Hakenes . * Adapted to 'libsi' for VDR 1.3.0 by Marcel Wiesweg . * - * $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 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 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());