Mailing List archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[vdr] Re: Aborted - a test case
> I think that any program mustn't relay on correct input data.
> Error checking has to be done, even if outer struct/crc indicates
> that everythin might be fine.
> This case leads to a abort() only because of the huge malloc, but
> in other cases the malloc size may be fine and corrupted data is
> feed to vdr.
> If I have 692 input bytes, the event text clearly cannot contain
> 40xx bytes. This can easily be checked, or not?
I agree, this has to be checked. It just has not been done till today, because
noone bothered and no one needed it. Now that we have the first case we will
fix it.
The patch attached tries to fix the problem without C++ exceptions. A validity
flag is used to indicate an error. As soon as the flag is set, the loop
structures will not return any more data, which should be enough to
effectively stop processing the section.
The size checks are done at a few "strategic" points, so this does not emulate
an MMU, and this is not vsftpd, but it seems to work. I put some thought in
it but I cannot promise anything.
An API change was required:
SI::EIT::Event SiEitEvent;
- for (SI::Loop::Iterator it; eventLoop.hasNext(it); ) {
+ for (SI::Loop::Iterator it; eventLoop.getNext(SiEitEvent, it); ) {
- SiEitEvent = eventLoop.getNext(it);
The patch contains the necessary changes for VDR. Tested for two hours on some
channels.
Marcel
>
> Regards.
diff -U 2 -r -x osd -x evice libsi/vdr-1.3.9/eit.c ./eit.c
--- libsi/vdr-1.3.9/eit.c Sat Mar 20 11:53:23 2004
+++ ./eit.c Wed Sep 15 19:27:21 2004
@@ -48,7 +48,6 @@
SI::EIT::Event SiEitEvent;
- for (SI::Loop::Iterator it; eventLoop.hasNext(it); ) {
+ for (SI::Loop::Iterator it; eventLoop.getNext(SiEitEvent, it); ) {
Empty = false;
- SiEitEvent = eventLoop.getNext(it);
cEvent *pEvent = (cEvent *)pSchedule->GetEvent(SiEitEvent.getEventId(), SiEitEvent.getStartTime());
diff -U 2 -r -x osd -x evice libsi/vdr-1.3.9/libsi/descriptor.c ./libsi/descriptor.c
--- libsi/vdr-1.3.9/libsi/descriptor.c Fri Mar 26 16:25:28 2004
+++ ./libsi/descriptor.c Wed Sep 15 19:14:08 2004
@@ -112,7 +112,5 @@
index += strlen(separation2);
ExtendedEventDescriptor::Item item;
- for (Loop::Iterator it; d->itemLoop.hasNext(it); ) {
- item=d->itemLoop.getNext(it);
-
+ for (Loop::Iterator it; d->itemLoop.getNext(item, it); ) {
item.itemDescription.getText(tempbuf);
len=strlen(tempbuf);
@@ -200,7 +198,5 @@
ExtendedEventDescriptor::Item item;
- for (Loop::Iterator it; d->itemLoop.hasNext(it); ) {
- item=d->itemLoop.getNext(it);
-
+ for (Loop::Iterator it; d->itemLoop.getNext(item, it); ) {
item.itemDescription.getText(tempbuf);
len=strlen(tempbuf);
@@ -242,7 +238,5 @@
ExtendedEventDescriptor::Item item;
- if (d->itemLoop.hasNext(it)) {
- item=d->itemLoop.getNext(it);
-
+ if (d->itemLoop.getNext(item, it)) {
item.item.getText(itemDescription);
item.itemDescription.getText(itemText);
Only in ./libsi: descriptor.o
Only in ./libsi: eit.bin
Only in ./libsi: libsi.a
Only in ./libsi: section.o
diff -U 2 -r -x osd -x evice libsi/vdr-1.3.9/libsi/si.c ./libsi/si.c
--- libsi/vdr-1.3.9/libsi/si.c Sat May 29 19:06:23 2004
+++ ./libsi/si.c Wed Sep 15 19:12:25 2004
@@ -31,4 +31,8 @@
}
+bool Object::checkSize(unsigned int offset) {
+ return data.checkSize(offset);
+}
+
Section::Section(const unsigned char *data, bool doCopy) {
setData(data, getLength(data), doCopy);
@@ -51,13 +55,13 @@
}
-bool CRCSection::isValid() {
+bool CRCSection::isCRCValid() {
return CRC32::isValid((const char *)data.getData(), getLength()/*, data.FourBytes(getLength()-4)*/);
}
bool CRCSection::CheckCRCAndParse() {
- if (!isValid())
+ if (!isCRCValid())
return false;
CheckParse();
- return true;
+ return isValid();
}
@@ -103,5 +107,5 @@
Descriptor *DescriptorLoop::getNext(Iterator &it) {
- if (it.i<getLength()) {
+ if (isValid() && it.i<getLength()) {
return createDescriptor(it.i, true);
}
@@ -111,5 +115,5 @@
Descriptor *DescriptorLoop::getNext(Iterator &it, DescriptorTag tag, bool returnUnimplemetedDescriptor) {
Descriptor *d=0;
- if (it.i<getLength()) {
+ if (isValid() && it.i<getLength()) {
const unsigned char *p=data.getData(it.i);
const unsigned char *end=p+getLength()-it.i;
@@ -129,5 +133,5 @@
Descriptor *DescriptorLoop::getNext(Iterator &it, DescriptorTag *tags, int arrayLength, bool returnUnimplementedDescriptor) {
Descriptor *d=0;
- if (it.i<getLength()) {
+ if (isValid() && it.i<getLength()) {
const unsigned char *p=data.getData(it.i);
const unsigned char *end=p+getLength()-it.i;
@@ -148,4 +152,6 @@
Descriptor *DescriptorLoop::createDescriptor(int &i, bool returnUnimplemetedDescriptor) {
+ if (!checkSize(Descriptor::getLength(data.getData(i))))
+ return 0;
Descriptor *d=Descriptor::getDescriptor(data+i, domain, returnUnimplemetedDescriptor);
if (!d)
diff -U 2 -r -x osd -x evice libsi/vdr-1.3.9/libsi/si.h ./libsi/si.h
--- libsi/vdr-1.3.9/libsi/si.h Sun Mar 7 11:09:49 2004
+++ ./libsi/si.h Wed Sep 15 19:34:08 2004
@@ -181,4 +181,6 @@
void setData(const unsigned char*data, unsigned 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(); }
virtual int getLength() = 0;
protected:
@@ -187,4 +189,7 @@
template <class T> friend class StructureLoop;
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);
};
@@ -206,5 +211,5 @@
CRCSection(const unsigned char *data, bool doCopy=true) : Section(data, doCopy) {}
CRCSection() {}
- bool isValid();
+ bool isCRCValid();
//convenience: isValid+CheckParse
bool CheckCRCAndParse();
@@ -230,7 +235,7 @@
public:
//never forget to call this
- void setData(CharArray d, int l) { Object::setData(d); length=l; }
+ 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); length=l; offset+=l; }
+ void setDataAndOffset(CharArray d, int l, unsigned int &offset) { Object::setData(d); checkSize(l); length=l; offset+=l; }
virtual int getLength() { return length; }
private:
@@ -282,6 +287,8 @@
//currently you must use a while-loop testing for hasNext()
//i must be 0 to get the first descriptor (with the first call)
- T getNext(Iterator &it)
+ bool getNext(T &obj, Iterator &it)
{
+ if (!isValid() || it.i >= getLength())
+ return false;
CharArray d=data;
d.addOffset(it.i);
@@ -289,10 +296,13 @@
ret.setData(d);
ret.CheckParse();
+ if (!checkSize(ret.getLength()))
+ return false;
it.i+=ret.getLength();
- return ret;
+ obj=ret;
+ return true;
}
T* getNextAsPointer(Iterator &it)
{
- if (getLength() <= it.i)
+ if (!isValid() || it.i >= getLength())
return 0;
CharArray d=data;
@@ -301,8 +311,10 @@
ret->setData(d);
ret->CheckParse();
+ if (!checkSize(ret->getLength()))
+ return 0;
it.i+=ret->getLength();
return ret;
}
- bool hasNext(Iterator &it) { return getLength() > it.i; }
+ //bool hasNext(Iterator &it) { return getLength() > it.i; }
};
@@ -386,5 +398,5 @@
return ret;
}
- bool hasNext(Iterator &it) { return getLength() > it.i; }
+ bool hasNext(Iterator &it) { return isValid() && (getLength() > it.i); }
};
Only in ./libsi: si.o
Only in ./libsi: test
Only in ./libsi: test.c
diff -U 2 -r -x osd -x evice libsi/vdr-1.3.9/libsi/util.c ./libsi/util.c
--- libsi/vdr-1.3.9/libsi/util.c Mon Dec 22 15:03:03 2003
+++ ./libsi/util.c Wed Sep 15 19:05:50 2004
@@ -89,7 +89,5 @@
}
-CharArray::Data::Data() : count_(1) {
- size=0;
- data=0;
+CharArray::Data::Data() : data(0), size(0), count_(1), valid(true) {
/*
lockingPid = 0;
diff -U 2 -r -x osd -x evice libsi/vdr-1.3.9/libsi/util.h ./libsi/util.h
--- libsi/vdr-1.3.9/libsi/util.h Mon Dec 22 15:07:41 2003
+++ ./libsi/util.h Wed Sep 15 19:11:35 2004
@@ -59,4 +59,7 @@
u_int32_t FourBytes(const unsigned 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))); }
+
void addOffset(unsigned int offset) { off+=offset; }
private:
@@ -87,8 +90,10 @@
unsigned int size;
- unsigned count_;
// count_ is the number of CharArray objects that point at this
// count_ must be initialized to 1 by all constructors
// (it starts as 1 since it is pointed to by the CharArray object that created it)
+ unsigned count_;
+
+ bool valid;
/*
Only in ./libsi: util.o
Only in ./libsi: vdr-1.3.9
Only in .: libsi-memory.patch
Only in .: libsi.exceptions
Only in .: lirc.o
Only in .: menu.o
Only in .: menuitems.o
diff -U 2 -r -x osd -x evice libsi/vdr-1.3.9/nit.c ./nit.c
--- libsi/vdr-1.3.9/nit.c Sat May 22 17:46:21 2004
+++ ./nit.c Wed Sep 15 19:19:42 2004
@@ -93,6 +93,6 @@
if (!Channels.Lock(true, 10))
return;
- for (SI::Loop::Iterator it; nit.transportStreamLoop.hasNext(it); ) {
- SI::NIT::TransportStream ts = nit.transportStreamLoop.getNext(it);
+ SI::NIT::TransportStream ts;
+ for (SI::Loop::Iterator it; nit.transportStreamLoop.getNext(ts, it); ) {
SI::Descriptor *d;
for (SI::Loop::Iterator it2; (d = ts.transportStreamDescriptors.getNext(it2)); ) {
diff -U 2 -r -x osd -x evice libsi/vdr-1.3.9/pat.c ./pat.c
--- libsi/vdr-1.3.9/pat.c Sun May 23 11:29:04 2004
+++ ./pat.c Wed Sep 15 19:18:00 2004
@@ -286,6 +286,5 @@
SI::PAT::Association assoc;
int Index = 0;
- for (SI::Loop::Iterator it; pat.associationLoop.hasNext(it); ) {
- assoc = pat.associationLoop.getNext(it);
+ for (SI::Loop::Iterator it; pat.associationLoop.getNext(assoc, it); ) {
if (!assoc.isNITPid()) {
if (Index++ == pmtIndex) {
@@ -333,6 +332,5 @@
int NumApids = 0;
int NumDpids = 0;
- for (SI::Loop::Iterator it; pmt.streamLoop.hasNext(it); ) {
- stream = pmt.streamLoop.getNext(it);
+ for (SI::Loop::Iterator it; pmt.streamLoop.getNext(stream, it); ) {
switch (stream.getStreamType()) {
case 1: // STREAMTYPE_11172_VIDEO
Only in .: pat.o
Only in .: player.o
Only in .: plugin.o
Only in .: rcu.o
Only in .: receiver.o
Only in .: recorder.o
Only in .: recording.o
Only in .: remote.o
Only in .: remux.o
Only in .: ringbuffer.o
diff -U 2 -r -x osd -x evice libsi/vdr-1.3.9/sdt.c ./sdt.c
--- libsi/vdr-1.3.9/sdt.c Sun Mar 7 11:46:08 2004
+++ ./sdt.c Wed Sep 15 19:20:43 2004
@@ -40,7 +40,5 @@
return;
SI::SDT::Service SiSdtService;
- for (SI::Loop::Iterator it; sdt.serviceLoop.hasNext(it); ) {
- SiSdtService = sdt.serviceLoop.getNext(it);
-
+ for (SI::Loop::Iterator it; sdt.serviceLoop.getNext(SiSdtService, it); ) {
cChannel *channel = Channels.GetByChannelID(tChannelID(Source(), sdt.getOriginalNetworkId(), sdt.getTransportStreamId(), SiSdtService.getServiceId()));
if (!channel)
@@ -101,6 +99,6 @@
case SI::NVODReferenceDescriptorTag: {
SI::NVODReferenceDescriptor *nrd = (SI::NVODReferenceDescriptor *)d;
- for (SI::Loop::Iterator it; nrd->serviceLoop.hasNext(it); ) {
- SI::NVODReferenceDescriptor::Service Service = nrd->serviceLoop.getNext(it);
+ SI::NVODReferenceDescriptor::Service Service;
+ for (SI::Loop::Iterator it; nrd->serviceLoop.getNext(Service, it); ) {
//printf(" %04X-%04X-%04X\n", Service.getOriginalNetworkId(), Service.getTransportStream(), Service.getServiceId());//XXX TODO
}
Only in .: sdt.o
Home |
Main Index |
Thread Index