Matthias Lenk wrote:

I was experimenting with VDR 1.3.27 and DVB-T reception and found an issue in channels.h. The alangs member of the class cChannel has MAXAPIDS elements. But in channels.c line 447 an element with index MAXAPIDS can be accessed. But the max index is of course MAXAPIDS - 1. This can have weird effects, so I suggest to increase the number of elements to MAXAPIDS + 1. The same is true for dlangs member of the cChannels class.

I agree. The ?pid lists are zero-terminated, so they are [MAX?PIDS + 1] sized. The ?langs arrays are accessed in parallel, so they need to be sized the same, just like in pat.c line 329.

An alternative would be to rewrite the initializing loop in cChannel::SetPids, because there is no need to copy the lang of the terminating 0 pid. (the current loop always copies the whole array instead of stopping at the terminating 0 pid)

The bug is currently just cosmetic, because the only out-of-bounds write access to alangs[MAXAPIDS] trashes dpids[0] which is overwritten in the next step. Same happens for dlangs[MAXDPIDS] and spids[0], where spids is generally un-used by now.

Cheers,

Udo

Hi Udo,

I just wanted to add some details. That crash happened when watching DVB-T in Berlin and I had a special channels.conf that I do not find right now unfortunately. Also, it was not on an x86 platform so this might come into play as well.

Thanks,

Matthias

On Thursday 21 July 2005 15:13, Matthias Lenk wrote:

Hi Udo,

Thanks for the quick reply. I don't think that this is just cosmetic. I had a crash caused by this, not directly but indirectly, so I suggest to fix this.

Thanks,

Matthias

On Thursday 21 July 2005 14:37, Udo Richter wrote:

...

Matthias Lenk wrote:

...

I was experimenting with VDR 1.3.27 and DVB-T reception and found an issue in channels.h. The alangs member of the class cChannel has MAXAPIDS elements. But in channels.c line 447 an element with index MAXAPIDS can be accessed. But the max index is of course MAXAPIDS - 1. This can have weird effects, so I suggest to increase the number of elements to MAXAPIDS + 1. The same is true for dlangs member of the cChannels class.

I agree. The ?pid lists are zero-terminated, so they are [MAX?PIDS + 1] sized. The ?langs arrays are accessed in parallel, so they need to be sized the same, just like in pat.c line 329.

An alternative would be to rewrite the initializing loop in cChannel::SetPids, because there is no need to copy the lang of the terminating 0 pid. (the current loop always copies the whole array instead of stopping at the terminating 0 pid)

The bug is currently just cosmetic, because the only out-of-bounds write access to alangs[MAXAPIDS] trashes dpids[0] which is overwritten in the next step. Same happens for dlangs[MAXDPIDS] and spids[0], where spids is generally un-used by now.

Cheers,

Udo