Fixed a possible buffer overflow in handling CA descriptors

CONTRIBUTORS

@@ -2929,6 +2929,8 @@ Lars Hanisch <dvb@flensrocker.de>
  for fixing a memory leak in case of broken Extended Event Descriptors
  for adding a 'const' version of cTimers::GetTimer()
  for fixing a typo in the description of cTimers::GetTimersRead()
+ for suggesting to use dynamic buffering in handling CA descriptors to avoid a
+ possible buffer overflow
 
 Alex Lasnier <alex@fepg.org>
  for adding tuning support for ATSC devices

HISTORY

@@ -8828,7 +8828,7 @@ Video Disk Recorder Revision History
 - Empty adaptation field TS packets are now skipped when recording (thanks to
   Christopher Reimer, based on the "AFFcleaner" by Stefan P<>schel).
 
-2016-12-22: Version 2.3.2
+2016-12-23: Version 2.3.2
 
 - Fixed a crash when deleting a recording (reported by Oliver Endriss).
 - Fixed an overflow of PIDs in a receiver (thanks to Robert Hannebauer).
@@ -8874,3 +8874,5 @@ Video Disk Recorder Revision History
 - Added a 'const' version of cTimers::GetTimer() (thanks to Lars Hanisch).
 - Fixed a typo in the description of cTimers::GetTimersRead() (thanks to Lars
   Hanisch).
+- Fixed a possible buffer overflow in handling CA descriptors (suggested by
+  Lars Hanisch).

ci.c

@@ -4,7 +4,7 @@
  * See the main source file 'vdr.c' for copyright information and
  * how to reach the author.
  *
- * $Id: ci.c 4.2 2015/09/05 11:45:19 kls Exp $
+ * $Id: ci.c 4.3 2016/12/23 14:00:45 kls Exp $
  */
 
 #include "ci.h"
@@ -756,9 +756,9 @@ class cCiCaPmt {
   friend class cCiConditionalAccessSupport;
 private:
   uint8_t cmdId;
-  int length;
   int esInfoLengthPos;
-  uint8_t capmt[2048]; ///< XXX is there a specified maximum?
+  cDynamicBuffer caDescriptors;
+  cDynamicBuffer capmt;
   int source;
   int transponder;
   int programNumber;
@@ -768,7 +768,7 @@ public:
   cCiCaPmt(uint8_t CmdId, int Source, int Transponder, int ProgramNumber, const int *CaSystemIds);
   uint8_t CmdId(void) { return cmdId; }
   void SetListManagement(uint8_t ListManagement);
-  uint8_t ListManagement(void) { return capmt[0]; }
+  uint8_t ListManagement(void) { return capmt.Get(0); }
   void AddPid(int Pid, uint8_t StreamType);
   };
 
@@ -784,55 +784,46 @@ cCiCaPmt::cCiCaPmt(uint8_t CmdId, int Source, int Transponder, int ProgramNumber
          caSystemIds[i] = CaSystemIds[i];
      }
   caSystemIds[i] = 0;
-  uint8_t caDescriptors[512];
-  int caDescriptorsLength = GetCaDescriptors(source, transponder, programNumber, caSystemIds, sizeof(caDescriptors), caDescriptors, 0);
-  length = 0;
-  capmt[length++] = CPLM_ONLY;
-  capmt[length++] = (ProgramNumber >> 8) & 0xFF;
-  capmt[length++] =  ProgramNumber       & 0xFF;
-  capmt[length++] = 0x01; // version_number, current_next_indicator - apparently vn doesn't matter, but cni must be 1
-  esInfoLengthPos = length;
-  capmt[length++] = 0x00; // program_info_length H (at program level)
-  capmt[length++] = 0x00; // program_info_length L
-  AddCaDescriptors(caDescriptorsLength, caDescriptors);
+  GetCaDescriptors(source, transponder, programNumber, caSystemIds, caDescriptors, 0);
+  capmt.Append(CPLM_ONLY);
+  capmt.Append((ProgramNumber >> 8) & 0xFF);
+  capmt.Append( ProgramNumber       & 0xFF);
+  capmt.Append(0x01); // version_number, current_next_indicator - apparently vn doesn't matter, but cni must be 1
+  esInfoLengthPos = capmt.Length();
+  capmt.Append(0x00); // program_info_length H (at program level)
+  capmt.Append(0x00); // program_info_length L
+  AddCaDescriptors(caDescriptors.Length(), caDescriptors.Data());
 }
 
 void cCiCaPmt::SetListManagement(uint8_t ListManagement)
 {
-  capmt[0] = ListManagement;
+  capmt.Set(0, ListManagement);
 }
 
 void cCiCaPmt::AddPid(int Pid, uint8_t StreamType)
 {
   if (Pid) {
-     uint8_t caDescriptors[512];
-     int caDescriptorsLength = GetCaDescriptors(source, transponder, programNumber, caSystemIds, sizeof(caDescriptors), caDescriptors, Pid);
-     //XXX buffer overflow check???
-     capmt[length++] = StreamType;
-     capmt[length++] = (Pid >> 8) & 0xFF;
-     capmt[length++] =  Pid       & 0xFF;
-     esInfoLengthPos = length;
-     capmt[length++] = 0x00; // ES_info_length H (at ES level)
-     capmt[length++] = 0x00; // ES_info_length L
-     AddCaDescriptors(caDescriptorsLength, caDescriptors);
+     GetCaDescriptors(source, transponder, programNumber, caSystemIds, caDescriptors, Pid);
+     capmt.Append(StreamType);
+     capmt.Append((Pid >> 8) & 0xFF);
+     capmt.Append( Pid       & 0xFF);
+     esInfoLengthPos = capmt.Length();
+     capmt.Append(0x00); // ES_info_length H (at ES level)
+     capmt.Append(0x00); // ES_info_length L
+     AddCaDescriptors(caDescriptors.Length(), caDescriptors.Data());
      }
 }
 
 void cCiCaPmt::AddCaDescriptors(int Length, const uint8_t *Data)
 {
   if (esInfoLengthPos) {
-     if (length + Length < int(sizeof(capmt))) {
      if (Length || cmdId == CPCI_QUERY) {
-           capmt[length++] = cmdId;
-           memcpy(capmt + length, Data, Length);
-           length += Length;
-           int l = length - esInfoLengthPos - 2;
-           capmt[esInfoLengthPos]     = (l >> 8) & 0xFF;
-           capmt[esInfoLengthPos + 1] =  l       & 0xFF;
+        capmt.Append(cmdId);
+        capmt.Append(Data, Length);
+        int l = capmt.Length() - esInfoLengthPos - 2;
+        capmt.Set(esInfoLengthPos,     (l >> 8) & 0xFF);
+        capmt.Set(esInfoLengthPos + 1,  l       & 0xFF);
         }
-        }
-     else
-        esyslog("ERROR: buffer overflow in CA descriptor");
      esInfoLengthPos = 0;
      }
   else
@@ -995,7 +986,7 @@ void cCiConditionalAccessSupport::SendPMT(cCiCaPmt *CaPmt)
 {
   if (CaPmt && state >= 2) {
      dbgprotocol("Slot %d: ==> Ca Pmt (%d) %d %d\n", Tc()->CamSlot()->SlotNumber(), SessionId(), CaPmt->ListManagement(), CaPmt->CmdId());
-     SendData(AOT_CA_PMT, CaPmt->length, CaPmt->capmt);
+     SendData(AOT_CA_PMT, CaPmt->capmt.Length(), CaPmt->capmt.Data());
      state = 4; // sent ca pmt
      }
 }

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 4.2 2016/12/22 11:42:23 kls Exp $
+ * $Id: pat.c 4.3 2016/12/23 14:02:07 kls Exp $
  */
 
 #include "pat.h"
@@ -81,7 +81,7 @@ public:
   bool Is(cCaDescriptors * CaDescriptors);
   bool Empty(void) { return caDescriptors.Count() == 0; }
   void AddCaDescriptor(SI::CaDescriptor *d, int EsPid);
-  int GetCaDescriptors(const int *CaSystemIds, int BufSize, uchar *Data, int EsPid);
+  void GetCaDescriptors(const int *CaSystemIds, cDynamicBuffer &Buffer, int EsPid);
   int GetCaPids(const int *CaSystemIds, int BufSize, int *Pids);
   const int GetPmtPid(void) { return pmtPid; };
   const int *CaIds(void) { return caIds; }
@@ -159,30 +159,20 @@ void cCaDescriptors::AddCaDescriptor(SI::CaDescriptor *d, int EsPid)
 // =0 - common CaDescriptor
 // <0 - all CaDescriptors regardless of type (old default)
 
-int cCaDescriptors::GetCaDescriptors(const int *CaSystemIds, int BufSize, uchar *Data, int EsPid)
+void cCaDescriptors::GetCaDescriptors(const int *CaSystemIds, cDynamicBuffer &Buffer, int EsPid)
 {
+  Buffer.Clear();
   if (!CaSystemIds || !*CaSystemIds)
-     return 0;
-  if (BufSize > 0 && Data) {
-     int length = 0;
+     return;
   for (cCaDescriptor *d = caDescriptors.First(); d; d = caDescriptors.Next(d)) {
       if (EsPid < 0 || d->EsPid() == EsPid) {
          const int *caids = CaSystemIds;
          do {
-               if (*caids == 0xFFFF || d->CaSystem() == *caids) {
-                  if (length + d->Length() <= BufSize) {
-                     memcpy(Data + length, d->Data(), d->Length());
-                     length += d->Length();
-                     }
-                  else
-                     return -1;
-                  }
+            if (*caids == 0xFFFF || d->CaSystem() == *caids)
+               Buffer.Append(d->Data(), d->Length());
             } while (*++caids);
          }
       }
-     return length;
-     }
-  return -1;
 }
 
 int cCaDescriptors::GetCaPids(const int *CaSystemIds, int BufSize, int *Pids)
@@ -219,7 +209,7 @@ public:
       // Returns 0 if this is an already known descriptor,
       // 1 if it is an all new descriptor with actual contents,
       // and 2 if an existing descriptor was changed.
-  int GetCaDescriptors(int Source, int Transponder, int ServiceId, const int *CaSystemIds, int BufSize, uchar *Data, int EsPid);
+  void GetCaDescriptors(int Source, int Transponder, int ServiceId, const int *CaSystemIds, cDynamicBuffer &Buffer, int EsPid);
   int GetCaPids(int Source, int Transponder, int ServiceId, const int *CaSystemIds, int BufSize, int *Pids);
   int GetPmtPid(int Source, int Transponder, int ServiceId);
   };
@@ -242,14 +232,15 @@ int cCaDescriptorHandler::AddCaDescriptors(cCaDescriptors *CaDescriptors)
   return CaDescriptors->Empty() ? 0 : 1;
 }
 
-int cCaDescriptorHandler::GetCaDescriptors(int Source, int Transponder, int ServiceId, const int *CaSystemIds, int BufSize, uchar *Data, int EsPid)
+void cCaDescriptorHandler::GetCaDescriptors(int Source, int Transponder, int ServiceId, const int *CaSystemIds, cDynamicBuffer &Buffer, int EsPid)
 {
   cMutexLock MutexLock(&mutex);
   for (cCaDescriptors *ca = First(); ca; ca = Next(ca)) {
-      if (ca->Is(Source, Transponder, ServiceId))
-         return ca->GetCaDescriptors(CaSystemIds, BufSize, Data, EsPid);
+      if (ca->Is(Source, Transponder, ServiceId)) {
+         ca->GetCaDescriptors(CaSystemIds, Buffer, EsPid);
+         break;
+         }
       }
-  return 0;
 }
 
 int cCaDescriptorHandler::GetCaPids(int Source, int Transponder, int ServiceId, const int *CaSystemIds, int BufSize, int *Pids)
@@ -274,9 +265,9 @@ int cCaDescriptorHandler::GetPmtPid(int Source, int Transponder, int ServiceId)
 
 cCaDescriptorHandler CaDescriptorHandler;
 
-int GetCaDescriptors(int Source, int Transponder, int ServiceId, const int *CaSystemIds, int BufSize, uchar *Data, int EsPid)
+void GetCaDescriptors(int Source, int Transponder, int ServiceId, const int *CaSystemIds, cDynamicBuffer &Buffer, int EsPid)
 {
-  return CaDescriptorHandler.GetCaDescriptors(Source, Transponder, ServiceId, CaSystemIds, BufSize, Data, EsPid);
+  CaDescriptorHandler.GetCaDescriptors(Source, Transponder, ServiceId, CaSystemIds, Buffer, EsPid);
 }
 
 int GetCaPids(int Source, int Transponder, int ServiceId, const int *CaSystemIds, int BufSize, int *Pids)

pat.h

@@ -4,7 +4,7 @@
  * See the main source file 'vdr.c' for copyright information and
  * how to reach the author.
  *
- * $Id: pat.h 3.4 2015/01/04 13:17:22 kls Exp $
+ * $Id: pat.h 4.1 2016/12/23 14:03:24 kls Exp $
  */
 
 #ifndef __PAT_H
@@ -38,14 +38,12 @@ public:
   void Trigger(int Sid = -1);
   };
 
-int GetCaDescriptors(int Source, int Transponder, int ServiceId, const int *CaSystemIds, int BufSize, uchar *Data, int EsPid);
+void GetCaDescriptors(int Source, int Transponder, int ServiceId, const int *CaSystemIds, cDynamicBuffer &Buffer, int EsPid);
          ///< Gets all CA descriptors for a given channel.
          ///< Copies all available CA descriptors for the given Source, Transponder and ServiceId
-         ///< into the provided buffer at Data (at most BufSize bytes). Only those CA descriptors
+         ///< into the provided buffer. Only those CA descriptors
          ///< are copied that match one of the given CA system IDs (or all of them, if CaSystemIds
          ///< is 0xFFFF).
-         ///< Returns the number of bytes copied into Data (0 if no CA descriptors are
-         ///< available), or -1 if BufSize was too small to hold all CA descriptors.
 
 int GetCaPids(int Source, int Transponder, int ServiceId, const int *CaSystemIds, int BufSize, int *Pids);
          ///< Gets all CA pids for a given channel.

tools.c

@@ -4,7 +4,7 @@
  * See the main source file 'vdr.c' for copyright information and
  * how to reach the author.
  *
- * $Id: tools.c 4.4 2015/09/10 13:17:55 kls Exp $
+ * $Id: tools.c 4.5 2016/12/23 14:03:40 kls Exp $
  */
 
 #include "tools.h"
@@ -2272,6 +2272,44 @@ void cListBase::Sort(void)
   free(a);
 }
 
+// --- cDynamicBuffer --------------------------------------------------------
+
+cDynamicBuffer::cDynamicBuffer(int InitialSize)
+{
+  initialSize = InitialSize;
+  buffer = NULL;
+  size = used = 0;
+}
+
+cDynamicBuffer::~cDynamicBuffer()
+{
+  free(buffer);
+}
+
+bool cDynamicBuffer::Realloc(int NewSize)
+{
+  if (size < NewSize) {
+     NewSize = max(NewSize, size ? size * 3 / 2 : initialSize); // increase size by at least 50%
+     if (uchar *NewBuffer = (uchar *)realloc(buffer, NewSize)) {
+        buffer = NewBuffer;
+        size = NewSize;
+        }
+     else {
+        esyslog("ERROR: out of memory");
+        return false;
+        }
+     }
+  return true;
+}
+
+void cDynamicBuffer::Append(const uchar *Data, int Length)
+{
+  if (Assert(used + Length)) {
+     memcpy(buffer + used, Data, Length);
+     used += Length;
+     }
+}
+
 // --- cHashBase -------------------------------------------------------------
 
 cHashBase::cHashBase(int Size)

tools.h

@@ -4,7 +4,7 @@
  * See the main source file 'vdr.c' for copyright information and
  * how to reach the author.
  *
- * $Id: tools.h 4.4 2016/12/13 12:13:46 kls Exp $
+ * $Id: tools.h 4.5 2016/12/23 13:56:35 kls Exp $
  */
 
 #ifndef __TOOLS_H
@@ -775,6 +775,26 @@ public:
   bool Load(const char *Directory, bool DirsOnly = false);
   };
 
+class cDynamicBuffer {
+private:
+  uchar *buffer;
+  int initialSize;
+  int size; // the total size of the buffer (bytes in memory)
+  int used; // the number of used bytes, starting at the beginning of the buffer
+  bool Realloc(int NewSize);
+  bool Assert(int NewSize) { return size < NewSize ? Realloc(NewSize) : true; } // inline for performance!
+public:
+  cDynamicBuffer(int InitialSize = 1024);
+  ~cDynamicBuffer();
+  void Append(const uchar *Data, int Length);
+  void Append(uchar Data) { if (Assert(used + 1)) buffer[used++] = Data; }
+  void Set(int Index, uchar Data) { if (Assert(Index + 1)) buffer[Index] = Data; }
+  uchar Get(int Index) { return Index < used ? buffer[Index] : 0; }
+  void Clear(void) { used = 0; }
+  uchar *Data(void) { return buffer; }
+  int Length(void) { return used; }
+  };
+
 class cHashObject : public cListObject {
   friend class cHashBase;
 private: