diff options
| -rw-r--r-- | CONTRIBUTORS | 3 | ||||
| -rw-r--r-- | HISTORY | 3 | ||||
| -rw-r--r-- | eit.c | 10 | ||||
| -rw-r--r-- | libsi/descriptor.c | 134 | ||||
| -rw-r--r-- | libsi/descriptor.h | 10 | ||||
| -rw-r--r-- | libsi/si.c | 33 | ||||
| -rw-r--r-- | libsi/si.h | 10 | ||||
| -rw-r--r-- | nit.c | 4 | ||||
| -rw-r--r-- | sdt.c | 4 |
9 files changed, 117 insertions, 94 deletions
diff --git a/CONTRIBUTORS b/CONTRIBUTORS index 4c2889b0..d6f34f1d 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -1021,3 +1021,6 @@ Marco Schlüßler <marco@lordzodiac.de> Jürgen Schmitz <j.schmitz@web.de> for reporting a bug in displaying the current channel when switching via the SVDRP command CHAN + +Philip Lawatsch <philip@lawatsch.at> + for debugging a buffer overflow in eit.c @@ -2889,3 +2889,6 @@ Video Disk Recorder Revision History - Added a note about the default assignment of the color keys to MANUAL. - Added a note about NPTL ("Native Posix Thread Library") to the INSTALL file (apparently the "fix" in version 1.3.0 didn't really fix this). +- Modified 'libsi' to require callers to state the buffer sizes when getting + strings in order to avoid buffer overflows (thanks to Philip Lawatsch for + debugging a buffer overflow in 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.94 2004/03/20 10:53:23 kls Exp $ + * $Id: eit.c 1.95 2004/06/06 14:49:45 kls Exp $ */ #include "eit.h" @@ -193,12 +193,12 @@ cEIT::cEIT(cSchedules *Schedules, int Source, u_char Tid, const u_char *Data) if (!rEvent) { if (ShortEventDescriptor) { char buffer[256]; - pEvent->SetTitle(ShortEventDescriptor->name.getText(buffer)); - pEvent->SetShortText(ShortEventDescriptor->text.getText(buffer)); + pEvent->SetTitle(ShortEventDescriptor->name.getText(buffer, sizeof(buffer))); + pEvent->SetShortText(ShortEventDescriptor->text.getText(buffer, sizeof(buffer))); } if (ExtendedEventDescriptors) { - char buffer[ExtendedEventDescriptors->getMaximumTextLength(": ")]; - pEvent->SetDescription(ExtendedEventDescriptors->getText(buffer, ": ")); + char buffer[ExtendedEventDescriptors->getMaximumTextLength(": ") + 1]; + pEvent->SetDescription(ExtendedEventDescriptors->getText(buffer, sizeof(buffer), ": ")); } } delete ExtendedEventDescriptors; diff --git a/libsi/descriptor.c b/libsi/descriptor.c index 685722e7..eb921c95 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.12 2004/03/26 15:25:28 kls Exp $ + * $Id: descriptor.c 1.13 2004/06/06 14:47:30 kls Exp $ * * ***************************************************************************/ @@ -84,53 +84,61 @@ int ExtendedEventDescriptors::getMaximumTextLength(const char *separation1, cons } char *ExtendedEventDescriptors::getText(const char *separation1, const char *separation2) { - char *text=new char[getMaximumTextLength(separation1, separation2)]; - return getText(text, separation1, separation2); + int size = getMaximumTextLength(separation1, separation2); + char *text=new char[size]; + return getText(text, size, separation1, separation2); } -char *ExtendedEventDescriptors::getText(char *buffer, const char *separation1, const char *separation2) { +char *ExtendedEventDescriptors::getText(char *buffer, int size, const char *separation1, const char *separation2) { int index=0, len; - char tempbuf[256]; for (int i=0;i<length;i++) { ExtendedEventDescriptor *d=(ExtendedEventDescriptor *)array[i]; if (!d) continue; - d->text.getText(tempbuf); - len=strlen(tempbuf); - if (len) { - memcpy(buffer+index, tempbuf, len); - index+=len; - } + d->text.getText(buffer+index, size); + len = strlen(buffer+index); + index += len; + size -= len; } + int sepLen1 = strlen(separation1); + int sepLen2 = strlen(separation2); + bool separated = false; for (int i=0;i<length;i++) { ExtendedEventDescriptor *d=(ExtendedEventDescriptor *)array[i]; if (!d) continue; - strcpy(buffer+index, separation2); // let's have a separator between the long text and the items - index += strlen(separation2); ExtendedEventDescriptor::Item item; for (Loop::Iterator it; d->itemLoop.hasNext(it); ) { item=d->itemLoop.getNext(it); - item.itemDescription.getText(tempbuf); - len=strlen(tempbuf); - if (len) { - memcpy(buffer+index, tempbuf, len); - index+=len; + if (!separated && size > sepLen2) { + strcpy(buffer+index, separation2); // let's have a separator between the long text and the items + index += sepLen2; + size -= sepLen2; + separated = true; + } + + item.itemDescription.getText(buffer+index, size); + len = strlen(buffer+index); + index += len; + size -= len; + if (size > sepLen1) { + strcpy(buffer+index, separation1); + index += sepLen1; + size -= sepLen1; } - strcpy(buffer+index, separation1); - index += strlen(separation1); - - item.item.getText(tempbuf); - len=strlen(tempbuf); - if (len) { - memcpy(buffer+index, tempbuf, len); - index+=len; + + item.item.getText(buffer+index, size); + len = strlen(buffer+index); + index += len; + size -= len; + if (size > sepLen2) { + strcpy(buffer+index, separation2); + index += sepLen2; + size -= sepLen2; } - strcpy(buffer+index, separation2); - index += strlen(separation2); } } @@ -150,23 +158,21 @@ int ExtendedEventDescriptors::getMaximumTextPlainLength() { } char *ExtendedEventDescriptors::getTextPlain() { - char *text=new char[getMaximumTextPlainLength()]; - return getTextPlain(text); + int size = getMaximumTextPlainLength(); + char *text=new char[size]; + return getTextPlain(text, size); } -char *ExtendedEventDescriptors::getTextPlain(char *buffer) { +char *ExtendedEventDescriptors::getTextPlain(char *buffer, int size) { int index=0, len; - char tempbuf[256]; for (int i=0;i<length;i++) { ExtendedEventDescriptor *d=(ExtendedEventDescriptor *)array[i]; if (!d) continue; - d->text.getText(tempbuf); - len=strlen(tempbuf); - if (len) { - memcpy(buffer+index, tempbuf, len); - index+=len; - } + d->text.getText(buffer+index, size); + len = strlen(buffer+index); + index += len; + size -= len; } buffer[index]='\0'; return buffer; @@ -174,25 +180,27 @@ char *ExtendedEventDescriptors::getTextPlain(char *buffer) { int ExtendedEventDescriptors::getMaximumTextItemizedLength(const char *separation1, const char *separation2) { int ret=0; - int sepLength=strlen(separation1)+strlen(separation2)-2; + int sepLength=strlen(separation1)+strlen(separation2); for (int i=0;i<length;i++) { ExtendedEventDescriptor *d=(ExtendedEventDescriptor *)array[i]; if (!d) continue; - //The length includes two 8-bit length fields which have already been subtracted from sepLength + //The length includes two 8-bit length fields which have already been subtracted from sepLength //XXX kls 2004-06-06: what does this mean??? ret+=d->itemLoop.getLength()+sepLength; } return ret; } char *ExtendedEventDescriptors::getTextItemized(const char *separation1, const char *separation2) { - char *text=new char[getMaximumTextItemizedLength(separation1, separation2)]; - return getTextItemized(text, separation1, separation2); + int size = getMaximumTextItemizedLength(separation1, separation2); + char *text=new char[size]; + return getTextItemized(text, size, separation1, separation2); } -char *ExtendedEventDescriptors::getTextItemized(char *buffer, const char *separation1, const char *separation2) { +char *ExtendedEventDescriptors::getTextItemized(char *buffer, int size, const char *separation1, const char *separation2) { int index=0, len; - char tempbuf[256]; + int sepLen1 = strlen(separation1); + int sepLen2 = strlen(separation2); for (int i=0;i<length;i++) { ExtendedEventDescriptor *d=(ExtendedEventDescriptor *)array[i]; if (!d) @@ -202,23 +210,25 @@ char *ExtendedEventDescriptors::getTextItemized(char *buffer, const char *separa for (Loop::Iterator it; d->itemLoop.hasNext(it); ) { item=d->itemLoop.getNext(it); - item.itemDescription.getText(tempbuf); - len=strlen(tempbuf); - if (len) { - memcpy(buffer+index, tempbuf, len); - index+=len; + item.itemDescription.getText(buffer+index, size); + len = strlen(buffer+index); + index += len; + size -= len; + if (size > sepLen1) { + strcpy(buffer+index, separation1); + index += sepLen1; + size -= sepLen1; } - strcpy(buffer+index, separation1); - index += strlen(separation1); - - item.item.getText(tempbuf); - len=strlen(tempbuf); - if (len) { - memcpy(buffer+index, tempbuf, len); - index+=len; + + item.item.getText(buffer+index, size); + len = strlen(buffer+index); + index += len; + size -= len; + if (size > sepLen2) { + strcpy(buffer+index, separation2); + index += sepLen2; + size -= sepLen2; } - strcpy(buffer+index, separation2); - index += strlen(separation2); } } buffer[index]='\0'; @@ -227,7 +237,7 @@ char *ExtendedEventDescriptors::getTextItemized(char *buffer, const char *separa //returns the itemized text pair by pair. Maximum length for buffers is 256. //Return value is false if and only if the end of the list is reached. -bool ExtendedEventDescriptors::getTextItemized(Loop::Iterator &it, bool &valid, char *itemDescription, char *itemText) { +bool ExtendedEventDescriptors::getTextItemized(Loop::Iterator &it, bool &valid, char *itemDescription, char *itemText, int sizeItemDescription, int sizeItemText) { //The iterator has to store two values: The descriptor index (4bit) //and the item loop index (max overall length 256, min item length 16 => max number 128 => 7bit) valid=false; @@ -244,8 +254,8 @@ bool ExtendedEventDescriptors::getTextItemized(Loop::Iterator &it, bool &valid, if (d->itemLoop.hasNext(it)) { item=d->itemLoop.getNext(it); - item.item.getText(itemDescription); - item.itemDescription.getText(itemText); + item.item.getText(itemDescription, sizeItemDescription); + item.itemDescription.getText(itemText, sizeItemText); valid=true; break; } else { diff --git a/libsi/descriptor.h b/libsi/descriptor.h index db3bba61..94959231 100644 --- a/libsi/descriptor.h +++ b/libsi/descriptor.h @@ -6,7 +6,7 @@ * the Free Software Foundation; either version 2 of the License, or * * (at your option) any later version. * * * - * $Id: descriptor.h 1.9 2004/03/26 15:26:03 kls Exp $ + * $Id: descriptor.h 1.10 2004/06/06 13:51:29 kls Exp $ * * ***************************************************************************/ @@ -55,12 +55,12 @@ public: //same semantics as with SI::String char *getText(const char *separation1="\t", const char *separation2="\n"); //buffer must at least be getTextLength(), getMaximumTextLength() is a good choice - char *getText(char *buffer, const char *separation1="\t", const char *separation2="\n"); + char *getText(char *buffer, int size, const char *separation1="\t", const char *separation2="\n"); //these only return the non-itemized text fields in concatenated form int getMaximumTextPlainLength(); char *getTextPlain(); - char *getTextPlain(char *buffer); + char *getTextPlain(char *buffer, int size); //these only return the itemized text fields in concatenated form. //Between the description and the text the separation1 character is used, @@ -68,11 +68,11 @@ public: //Director\tSteven Spielberg\nActor\tMichael Mendl\n int getMaximumTextItemizedLength(const char *separation1="\t", const char *separation2="\n"); char *getTextItemized(const char *separation1="\t", const char *separation2="\n"); - char *getTextItemized(char *buffer, const char *separation1="\t", const char *separation2="\n"); + char *getTextItemized(char *buffer, int size, const char *separation1="\t", const char *separation2="\n"); //returns the itemized text pair by pair. Maximum length for buffers is 256. //Return value is false if and only if the end of the list is reached. //The argument valid indicates whether the buffers contain valid content. - bool getTextItemized(Loop::Iterator &it, bool &valid, char *itemDescription, char *itemText); + bool getTextItemized(Loop::Iterator &it, bool &valid, char *itemDescription, char *itemText, int sizeItemDescription, int sizeItemText); }; class TimeShiftedEventDescriptor : public Descriptor { @@ -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.10 2004/05/29 17:06:23 kls Exp $ + * $Id: si.c 1.11 2004/06/06 14:43:56 kls Exp $ * * ***************************************************************************/ @@ -206,33 +206,36 @@ bool DescriptorGroup::isComplete() { char *String::getText() { if (getLength() < 0 || getLength() >4095) - return "text error"; + return strdup("text error"); // caller will delete it! char *data=new char(getLength()+1); - decodeText(data); + decodeText(data, getLength()+1); return data; } -char *String::getText(char *buffer) { - if (getLength() < 0 || getLength() >4095) { - strncpy(buffer, "text error", getLength()+1); +char *String::getText(char *buffer, int size) { + if (getLength() < 0 || getLength() >= size) { + strncpy(buffer, "text error", size); + buffer[size-1] = 0; return buffer; } - decodeText(buffer); + decodeText(buffer, size); return buffer; } //taken from VDR, Copyright Klaus Schmidinger <kls@cadsoft.de> -char *String::getText(char *buffer, char *shortVersion) { - if (getLength() < 0 || getLength() >4095) { - strncpy(buffer, "text error", getLength()+1); +char *String::getText(char *buffer, char *shortVersion, int sizeBuffer, int sizeShortVersion) { + if (getLength() < 0 || getLength() >= sizeBuffer) { + strncpy(buffer, "text error", sizeBuffer); + buffer[sizeBuffer-1] = 0; + *shortVersion = 0; return buffer; } - decodeText(buffer, shortVersion); + decodeText(buffer, shortVersion, sizeBuffer, sizeShortVersion); return buffer; } //taken from libdtv, Copyright Rolf Hakenes <hakenes@hippomi.de> -void String::decodeText(char *buffer) { +void String::decodeText(char *buffer, int size) { const unsigned char *from=data.getData(0); char *to=buffer; @@ -254,11 +257,13 @@ void String::decodeText(char *buffer) { else if (*from == 0x8A) *to++ = '\n'; from++; + if (to - buffer >= size - 1) + break; } *to = '\0'; } -void String::decodeText(char *buffer, char *shortVersion) { +void String::decodeText(char *buffer, char *shortVersion, int sizeBuffer, int sizeShortVersion) { const unsigned char *from=data.getData(0); char *to=buffer; char *toShort=shortVersion; @@ -283,6 +288,8 @@ void String::decodeText(char *buffer, char *shortVersion) { else if (*from == 0x87) IsShortName--; from++; + if (to - buffer >= sizeBuffer - 1 || toShort - shortVersion >= sizeShortVersion - 1) + break; } *to = '\0'; *toShort = '\0'; @@ -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.9 2004/03/07 10:09:49 kls Exp $ + * $Id: si.h 1.10 2004/06/06 13:35:21 kls Exp $ * * ***************************************************************************/ @@ -431,18 +431,18 @@ public: //so the maximum there is 256. //returns the given buffer for convenience. //The emphasis marks 0x86 and 0x87 are still available. - char *getText(char *buffer); + char *getText(char *buffer, int size); //The same semantics as for getText(char*) apply. //The short version of the text according to ETSI TR 101 211 (chapter 4.6) //will be written into the shortVersion buffer (which should, therefore, have the same //length as buffer). If no shortVersion is available, shortVersion will contain //an empty string. //The emphasis marks 0x86 and 0x87 are still available in buffer, but not in shortVersion. - char *getText(char *buffer, char *shortVersion); + char *getText(char *buffer, char *shortVersion, int sizeBuffer, int sizeShortVersion); protected: virtual void Parse() {} - void decodeText(char *buffer); - void decodeText(char *buffer, char *shortVersion); + void decodeText(char *buffer, int size); + void decodeText(char *buffer, char *shortVersion, int sizeBuffer, int sizeShortVersion); }; } //end of namespace @@ -4,7 +4,7 @@ * See the main source file 'vdr.c' for copyright information and * how to reach the author. * - * $Id: nit.c 1.7 2004/05/22 15:46:21 kls Exp $ + * $Id: nit.c 1.8 2004/06/06 14:24:49 kls Exp $ */ #include "nit.h" @@ -71,7 +71,7 @@ void cNitFilter::Process(u_short Pid, u_char Tid, const u_char *Data, int Length switch (d->getDescriptorTag()) { case SI::NetworkNameDescriptorTag: { SI::NetworkNameDescriptor *nnd = (SI::NetworkNameDescriptor *)d; - nnd->name.getText(nits[numNits].name); + nnd->name.getText(nits[numNits].name, MAXNETWORKNAME); } break; default: ; @@ -4,7 +4,7 @@ * See the main source file 'vdr.c' for copyright information and * how to reach the author. * - * $Id: sdt.c 1.8 2004/03/07 10:46:08 kls Exp $ + * $Id: sdt.c 1.9 2004/06/06 14:25:22 kls Exp $ */ #include "sdt.h" @@ -59,7 +59,7 @@ void cSdtFilter::Process(u_short Pid, u_char Tid, const u_char *Data, int Length { char NameBuf[1024]; char ShortNameBuf[1024]; - sd->serviceName.getText(NameBuf, ShortNameBuf); + sd->serviceName.getText(NameBuf, ShortNameBuf, sizeof(NameBuf), sizeof(ShortNameBuf)); char *pn = compactspace(NameBuf); char *ps = compactspace(ShortNameBuf); if (*NameBuf && *ShortNameBuf && strcmp(NameBuf, ShortNameBuf) != 0) { |