Modified 'libsi' to require callers to state the buffer sizes when getting strings in order to avoid buffer overflows

This commit is contained in:
Klaus Schmidinger 2004-06-06 14:53:21 +02:00
parent 125f3fe07b
commit def0c7aaa0
9 changed files with 115 additions and 92 deletions

View File

@ -1021,3 +1021,6 @@ Marco Schl
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

View File

@ -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).

10
eit.c
View File

@ -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;

View File

@ -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;
}
strcpy(buffer+index, separation1);
index += strlen(separation1);
item.item.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;
}
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 {

View File

@ -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 {

View File

@ -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';

View File

@ -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
nit.c
View File

@ -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
sdt.c
View File

@ -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) {