From 68acf8815c369e1bedfc0fa8f066975001803454 Mon Sep 17 00:00:00 2001 From: Klaus Schmidinger Date: Fri, 23 Dec 2016 14:08:14 +0100 Subject: [PATCH] Fixed a possible buffer overflow in handling CA descriptors --- CONTRIBUTORS | 2 ++ HISTORY | 4 +++- ci.c | 67 +++++++++++++++++++++++----------------------------- pat.c | 51 ++++++++++++++++----------------------- pat.h | 8 +++---- tools.c | 40 ++++++++++++++++++++++++++++++- tools.h | 22 ++++++++++++++++- 7 files changed, 118 insertions(+), 76 deletions(-) diff --git a/CONTRIBUTORS b/CONTRIBUTORS index c1b62fc3..51507ea2 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -2929,6 +2929,8 @@ Lars Hanisch 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 for adding tuning support for ATSC devices diff --git a/HISTORY b/HISTORY index 1ea29322..e378dc67 100644 --- a/HISTORY +++ b/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). diff --git a/ci.c b/ci.c index f86f668e..606875b8 100644 --- a/ci.c +++ b/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; - } + if (Length || cmdId == CPCI_QUERY) { + 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 } } diff --git a/pat.c b/pat.c index a22c9ac9..e40d2d5f 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 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; - 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; - } - } while (*++caids); - } + 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) + 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) diff --git a/pat.h b/pat.h index 19e60dcf..b9929754 100644 --- a/pat.h +++ b/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. diff --git a/tools.c b/tools.c index adfff1c3..754673db 100644 --- a/tools.c +++ b/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) diff --git a/tools.h b/tools.h index f2ba3157..73cca5a3 100644 --- a/tools.h +++ b/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: