From patchwork Sat Feb 18 12:35:30 2006
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
X-Patchwork-Submitter: Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de>
X-Patchwork-Id: 12207
Received: from raven.cadsoft.de ([217.7.101.211])
	by www.linuxtv.org with esmtp (Exim 4.50) id 1FARJG-0001Rz-Ll
	for vdr@linuxtv.org; Sat, 18 Feb 2006 13:35:34 +0100
Received: from [192.168.100.10] (hawk.cadsoft.de [192.168.100.10])
	by raven.cadsoft.de (8.13.3/8.13.3) with ESMTP id k1ICZWaO025837
	for <vdr@linuxtv.org>; Sat, 18 Feb 2006 13:35:32 +0100
Message-ID: <43F71492.1040801@cadsoft.de>
Date: Sat, 18 Feb 2006 13:35:30 +0100
From: Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de>
Organization: CadSoft Computer GmbH
User-Agent: Mozilla Thunderbird 1.0.6 (X11/20050716)
X-Accept-Language: en
MIME-Version: 1.0
To: vdr@linuxtv.org
Subject: Re: [vdr] Vdr core dump when tuning to FR3
References: <87oe176ul3.fsf@gandalf.hd.free.fr>
In-Reply-To: <87oe176ul3.fsf@gandalf.hd.free.fr>
X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-2.0
	(raven.cadsoft.de [192.168.1.1]);
	Sat, 18 Feb 2006 13:35:34 +0100 (CET)
X-BeenThere: vdr@linuxtv.org
X-Mailman-Version: 2.1.5
Precedence: list
Reply-To: VDR Mailing List <vdr@linuxtv.org>
List-Id: VDR Mailing List <vdr.linuxtv.org>
List-Unsubscribe: <http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr>,
	<mailto:vdr-request@linuxtv.org?subject=unsubscribe>
List-Archive: <http://www.linuxtv.org/pipermail/vdr>
List-Post: <mailto:vdr@linuxtv.org>
List-Help: <mailto:vdr-request@linuxtv.org?subject=help>
List-Subscribe: <http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr>,
	<mailto:vdr-request@linuxtv.org?subject=subscribe>
X-List-Received-Date: Sat, 18 Feb 2006 12:35:34 -0000
Status: O
X-Status: 
X-Keywords: 
X-UID: 7970

The attached patch (against VDR 1.3.42) should fix this.
It needs to be applied to the 'libsi' subdirectory.
After that, do a 'make clean' and 'make' in the VDR directory.


- Changed offset and size handling in 'libsi' from 'unsigned' to 'signed', so that
   overflows can be better detected (thanks to Marcel Wiesweg).
- Checking data size in CaDescriptor::Parse() and LinkageDescriptor::Parse() of
   'libsi' to avoid crashes with invalid data (thanks to Marcel Wiesweg).
- Made CharArray::DataOwnData::assign() in 'libsi' more robust against invalid
   data (suggested by Oliver Endriss). Also changed CharArray::DataOwnData::Delete()
   so that it sets 'size' and 'data' to 0.

Thanks also to Helmut Auer for testing and debugging.

Klaus

--- descriptor.c	2005/09/03 15:16:49	1.15
+++ descriptor.c	2006/02/18 11:02:25
@@ -16,7 +16,7 @@
 namespace SI {
 
 void ShortEventDescriptor::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    const descr_short_event *s;
    data.setPointerAndOffset<const descr_short_event>(s, offset);
    languageCode[0]=s->lang_code1;
@@ -38,7 +38,7 @@
 }
 
 void ExtendedEventDescriptor::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const descr_extended_event>(s, offset);
    languageCode[0]=s->lang_code1;
    languageCode[1]=s->lang_code2;
@@ -51,7 +51,7 @@
 }
 
 void ExtendedEventDescriptor::Item::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    const item_extended_event *first;
    data.setPointerAndOffset<const item_extended_event>(first, offset);
    itemDescription.setDataAndOffset(data+offset, first->item_description_length, offset);
@@ -327,9 +327,12 @@
 }
 
 void CaDescriptor::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const descr_ca>(s, offset);
-   privateData.assign(data.getData(offset), getLength()-offset);
+   if (checkSize(getLength()-offset))
+      privateData.assign(data.getData(offset), getLength()-offset);
+   else
+      privateData.assign(NULL, 0);
 }
 
 int StreamIdentifierDescriptor::getComponentTag() const {
@@ -477,7 +480,7 @@
 }
 
 void ServiceDescriptor::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const descr_service>(s, offset);
    providerName.setDataAndOffset(data+offset, s->provider_name_length, offset);
    const descr_service_mid *mid;
@@ -526,7 +529,7 @@
 }
 
 void ComponentDescriptor::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const descr_component>(s, offset);
    languageCode[0]=s->lang_code1;
    languageCode[1]=s->lang_code2;
@@ -580,7 +583,7 @@
 }
 
 void FrequencyListDescriptor::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const descr_frequency_list>(s, offset);
    frequencies.setData(data+offset, getLength()-offset);
 }
@@ -594,7 +597,7 @@
 }
 
 void MultilingualNameDescriptor::Name::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    const entry_multilingual_name *s;
    data.setPointerAndOffset<const entry_multilingual_name>(s, offset);
    languageCode[0]=s->lang_code1;
@@ -609,7 +612,7 @@
 }
 
 void MultilingualComponentDescriptor::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const descr_multilingual_component>(s, offset);
    nameLoop.setData(data+sizeof(descr_multilingual_component), getLength()-sizeof(descr_multilingual_component));
 }
@@ -619,7 +622,7 @@
 }
 
 void MultilingualServiceNameDescriptor::Name::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    const entry_multilingual_name *s;
    data.setPointerAndOffset<const entry_multilingual_name>(s, offset);
    languageCode[0]=s->lang_code1;
@@ -633,9 +636,12 @@
 }
 
 void LinkageDescriptor::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const descr_linkage>(s, offset);
-   privateData.assign(data.getData(offset), getLength()-offset);
+   if (checkSize(getLength()-offset))
+      privateData.assign(data.getData(offset), getLength()-offset);
+   else
+      privateData.assign(NULL, 0);
 }
 
 int LinkageDescriptor::getTransportStreamId() const {
@@ -682,7 +688,7 @@
 }
 
 void PDCDescriptor::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const descr_pdc>(s, offset);
 }
 
@@ -731,7 +737,7 @@
 }
 
 void MHP_ApplicationDescriptor::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    const descr_application *dapp;
    data.setPointerAndOffset<const descr_application>(dapp, offset);
    profileLoop.setDataAndOffset(data+offset, dapp->application_profiles_length, offset);
@@ -790,7 +796,7 @@
 }
 
 void MHP_TransportProtocolDescriptor::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const descr_transport_protocol>(s, offset);
    if (getProtocolId() == ObjectCarousel) {
       const transport_via_oc *oc;
@@ -821,7 +827,7 @@
 }
 
 void MHP_DVBJApplicationLocationDescriptor::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    const descr_dvbj_application_location *first;
    data.setPointerAndOffset<const descr_dvbj_application_location>(first, offset);
    baseDirectory.setDataAndOffset(data+offset, first->base_directory_length, offset);
@@ -836,7 +842,7 @@
 }
 
 void MHP_ApplicationIconsDescriptor::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    const descr_application_icons_descriptor *first;
    data.setPointerAndOffset<const descr_application_icons_descriptor>(first, offset);
    iconLocator.setDataAndOffset(data+offset, first->icon_locator_length, offset);
--- section.c	2004/02/20 13:44:59	1.3
+++ section.c	2006/02/18 10:38:20
@@ -18,7 +18,7 @@
 /*********************** PAT ***********************/
 
 void PAT::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const pat>(s, offset);
    associationLoop.setData(data+offset, getLength()-offset-4);
 }
@@ -48,7 +48,7 @@
 /*********************** PMT ***********************/
 
 void PMT::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const pmt>(s, offset);
    commonDescriptors.setDataAndOffset(data+offset, HILO(s->program_info_length), offset);
    streamLoop.setData(data+offset, getLength()-offset-4);
@@ -71,7 +71,7 @@
 }
 
 void PMT::Stream::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const pmt_info>(s, offset);
    streamDescriptors.setData(data+offset, HILO(s->ES_info_length));
 }
@@ -79,7 +79,7 @@
 /*********************** TSDT ***********************/
 
 void TSDT::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const tsdt>(s, offset);
    transportStreamDescriptors.setDataAndOffset(data+offset, getLength()-offset-4, offset);
 }
@@ -91,7 +91,7 @@
 }
 
 void NIT::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const nit>(s, offset);
    commonDescriptors.setDataAndOffset(data+offset, HILO(s->network_descriptor_length), offset);
    const nit_mid *mid;
@@ -108,7 +108,7 @@
 }
 
 void NIT::TransportStream::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const ni_ts>(s, offset);
    transportStreamDescriptors.setData(data+offset, HILO(s->transport_descriptors_length));
 }
@@ -116,7 +116,7 @@
 /*********************** SDT ***********************/
 
 void SDT::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const sdt>(s, offset);
    serviceLoop.setData(data+offset, getLength()-offset-4); //4 is for CRC
 }
@@ -150,7 +150,7 @@
 }
 
 void SDT::Service::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const sdt_descr>(s, offset);
    serviceDescriptors.setData(data+offset, HILO(s->descriptors_loop_length));
 }
@@ -188,7 +188,7 @@
 }
 
 void EIT::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const eit>(s, offset);
    //printf("%d %d %d %d %d\n", getServiceId(), getTransportStreamId(), getOriginalNetworkId(), isPresentFollowing(), isActualTS());
    eventLoop.setData(data+offset, getLength()-offset-4); //4 is for CRC
@@ -243,7 +243,7 @@
 }
 
 void EIT::Event::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const eit_event>(s, offset);
    //printf("%d %d %d\n", getStartTime(), getDuration(), getRunningStatus());
    eventDescriptors.setData(data+offset, HILO(s->descriptors_loop_length));
@@ -266,7 +266,7 @@
 }
 
 void TOT::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const tot>(s, offset);
    descriptorLoop.setData(data+offset, getLength()-offset-4);
 }
@@ -274,7 +274,7 @@
 /*********************** RST ***********************/
 
 void RST::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    const rst *s;
    data.setPointerAndOffset<const rst>(s, offset);
    infoLoop.setData(data+offset, getLength()-offset);
@@ -315,7 +315,7 @@
 }
 
 void AIT::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const ait>(first, offset);
    commonDescriptors.setDataAndOffset(data+offset, HILO(first->common_descriptors_length), offset);
    const ait_mid *mid;
@@ -336,7 +336,7 @@
 }
 
 void AIT::Application::Parse() {
-   unsigned int offset=0;
+   int offset=0;
    data.setPointerAndOffset<const ait_app>(s, offset);
    applicationDescriptors.setData(data+offset, HILO(s->application_descriptors_length));
 }
--- si.c	2005/05/28 14:11:16	1.14
+++ si.c	2006/02/18 10:38:20
@@ -22,7 +22,7 @@
 Object::Object(CharArray &d) : data(d) {
 }
 
-void Object::setData(const unsigned char*d, unsigned int size, bool doCopy) {
+void Object::setData(const unsigned char*d, int size, bool doCopy) {
    data.assign(d, size, doCopy);
 }
 
@@ -30,7 +30,7 @@
    data=d;
 }
 
-bool Object::checkSize(unsigned int offset) {
+bool Object::checkSize(int offset) {
    return data.checkSize(offset);
 }
 
--- si.h	2005/09/03 15:19:00	1.12
+++ si.h	2006/02/18 10:38:20
@@ -184,7 +184,7 @@
    Object();
    Object(CharArray &d);
    //can only be called once since data is immutable
-   void setData(const unsigned char*data, unsigned int size, bool doCopy=true);
+   void setData(const unsigned char*data, 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(); }
@@ -196,7 +196,7 @@
    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);
+   bool checkSize(int offset);
 };
 
 class Section : public Object {
@@ -242,7 +242,7 @@
    //never forget to call this
    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); checkSize(l); length=l; offset+=l; }
+   void setDataAndOffset(CharArray d, int l, int &offset) { Object::setData(d); checkSize(l); length=l; offset+=l; }
    virtual int getLength() { return length; }
 private:
    int length;
@@ -384,7 +384,7 @@
 template <typename T> class TypeLoop : public Loop {
 public:
    int getCount() { return getLength()/sizeof(T); }
-   T operator[](const unsigned int index) const
+   T operator[](const int index) const
       {
          switch (sizeof(T)) {
          case 1:
--- util.c	2005/05/28 14:15:29	1.5
+++ util.c	2006/02/18 11:17:50
@@ -47,7 +47,7 @@
     return *this;
 }
 
-void CharArray::assign(const unsigned char*data, unsigned int size, bool doCopy) {
+void CharArray::assign(const unsigned char*data, int size, bool doCopy) {
     //immutable
     if (!data_)
       data_= doCopy ? (Data*)new DataOwnData() : (Data*)new DataForeignData();
@@ -76,13 +76,13 @@
       return false;
 
    //do _not_ use strcmp! Data is not necessarily null-terminated.
-   for (unsigned int i=0;i<data_->size;i++)
+   for (int i=0;i<data_->size;i++)
       if (data_->data[i] != other.data_->data[i])
          return false;
    return true;
 }
 
-CharArray CharArray::operator+(const unsigned int offset) const {
+CharArray CharArray::operator+(const int offset) const {
    CharArray f(*this);
    f.off+=offset;
    return f;
@@ -117,8 +117,10 @@
    Delete();
 }
 
-void CharArray::DataOwnData::assign(const unsigned char*d, unsigned int s) {
+void CharArray::DataOwnData::assign(const unsigned char*d, int s) {
    Delete();
+   if (!d || s > 100000 || s <= 0) // ultimate plausibility check
+      return;
    size=s;
    unsigned char *newdata=new unsigned char[size];
    memcpy(newdata, d, size);
@@ -127,13 +129,15 @@
 
 void CharArray::DataOwnData::Delete() {
    delete[] data;
+   size=0;
+   data=0;
 }
 
 CharArray::DataForeignData::~DataForeignData() {
    Delete();
 }
 
-void CharArray::DataForeignData::assign(const unsigned char*d, unsigned int s) {
+void CharArray::DataForeignData::assign(const unsigned char*d, int s) {
    size=s;
    data=d;
 }
@@ -143,7 +147,7 @@
 }
 
 /*
-void CharArray::Data::assign(unsigned int s) {
+void CharArray::Data::assign(int s) {
    if (data)
       delete[] data;
    size=s;
--- util.h	2004/10/23 14:22:40	1.5
+++ util.h	2006/02/18 10:38:20
@@ -37,14 +37,14 @@
    ~CharArray();
 
    //can be called exactly once
-   void assign(const unsigned char*data, unsigned int size, bool doCopy=true);
+   void assign(const unsigned char*data, int size, bool doCopy=true);
    //compares to a null-terminated string
    bool operator==(const char *string) const;
    //compares to another CharArray (data not necessarily null-terminated)
    bool operator==(const CharArray &other) const;
 
    //returns another CharArray with its offset incremented by offset
-   CharArray operator+(const unsigned int offset) const;
+   CharArray operator+(const int offset) const;
 
    //access and convenience methods
    const unsigned char* getData() const { return data_->data+off; }
@@ -52,28 +52,28 @@
    template <typename T> const T* getData() const { return (T*)(data_->data+off); }
    template <typename T> const T* getData(int offset) const { return (T*)(data_->data+offset+off); }
       //sets p to point to data+offset, increments offset
-   template <typename T> void setPointerAndOffset(const T* &p, unsigned int &offset) const { p=(T*)getData(offset); offset+=sizeof(T); }
-   unsigned char operator[](const unsigned int index) const { return data_->data ? data_->data[off+index] : 0; }
+   template <typename T> void setPointerAndOffset(const T* &p, int &offset) const { p=(T*)getData(offset); offset+=sizeof(T); }
+   unsigned char operator[](const int index) const { return data_->data ? data_->data[off+index] : 0; }
    int getLength() const { return data_->size; }
-   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; }
+   u_int16_t TwoBytes(const int index) const { return data_->data ? data_->TwoBytes(off+index) : 0; }
+   u_int32_t FourBytes(const 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))); }
+   bool checkSize(int offset) { return (data_->valid && offset>=0 && (data_->valid=(off+offset < data_->size))); }
 
-   void addOffset(unsigned int offset) { off+=offset; }
+   void addOffset(int offset) { off+=offset; }
 private:
    class Data {
    public:
       Data();
       virtual ~Data();
 
-      virtual void assign(const unsigned char*data, unsigned int size) = 0;
+      virtual void assign(const unsigned char*data, int size) = 0;
       virtual void Delete() = 0;
 
-      u_int16_t TwoBytes(const unsigned int index) const
+      u_int16_t TwoBytes(const int index) const
          { return (data[index] << 8) | data[index+1]; }
-      u_int32_t FourBytes(const unsigned int index) const
+      u_int32_t FourBytes(const int index) const
          { return (data[index] << 24) | (data[index+1] << 16) | (data[index+2] << 8) | data[index+3]; }
       /*#ifdef CHARARRAY_THREADSAFE
       void Lock();
@@ -83,11 +83,11 @@
       void Unlock() {}
       #endif
       Data(const Data& d);
-      void assign(unsigned int size);
+      void assign(int size);
       */
 
       const unsigned char*data;
-      unsigned int size;
+      int size;
 
       // count_ is the number of CharArray objects that point at this
       // count_ must be initialized to 1 by all constructors
@@ -106,18 +106,18 @@
    public:
       DataOwnData() {}
       virtual ~DataOwnData();
-      virtual void assign(const unsigned char*data, unsigned int size);
+      virtual void assign(const unsigned char*data, int size);
       virtual void Delete();
    };
    class DataForeignData : public Data {
    public:
       DataForeignData() {}
       virtual ~DataForeignData();
-      virtual void assign(const unsigned char*data, unsigned int size);
+      virtual void assign(const unsigned char*data, int size);
       virtual void Delete();
    };
    Data* data_;
-   unsigned int off;
+   int off;
 };