Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#4918 closed defect (fixed)

can overrun fixed sized array _tsdata

Reported by: Erik Hovland <erik@…> Owned by: Janne Grunau
Priority: trivial Milestone: 0.22
Component: mythtv Version: head
Severity: low Keywords:
Cc: Ticket locked: no

Description

The array _tsdata is a fixed size of 4 in class TSHeader. But the member function AFCOffset() indexes it at 4. This means that if this function is called and the conditional evaluates true that the array will be overrun. I am pretty sure this is a typo and it should just be _tsdata[3] instead of 4.

Attachments (4)

libs_libmythtv_mpeg_tspacket.h-dont-overrun-_tsdata.2.patch (731 bytes) - added by Erik Hovland <erik@…> 12 years ago.
Changes indexing value from 4 to 3 since the array is only four entries big
libs_libmythtv_mpeg_tspacket.h-move-AFCOffset-to-TSPacket.patch (1.3 KB) - added by Erik Hovland <erik@…> 12 years ago.
Moves AFCOffset to TSPacket so that we don't have funny indexing of _tsdata
libs_libmythtv_mpeg_tspacket.h-init-_tspayload.patch (650 bytes) - added by Erik Hovland <erik@…> 12 years ago.
init tspayload in ctor (so it does not have uninitialized values)
libs_libmythtv_mpeg_tspacket.h-comment-fixing.patch (1.2 KB) - added by Erik Hovland <erik@…> 12 years ago.
fix two spelling typos in the comments

Download all attachments as: .zip

Change History (7)

Changed 12 years ago by Erik Hovland <erik@…>

Changes indexing value from 4 to 3 since the array is only four entries big

comment:1 Changed 12 years ago by Janne Grunau

Milestone: unknown0.22
Owner: changed from Isaac Richards to Janne Grunau
Priority: minortrivial
Status: newaccepted

No, it's not an typo. It's correct to read the first byte after header which contains the adpation field length if the packet has one.

The function should be moved from TSHeader to TSPacket.

It won't cause problems since our TSPackets are continuous memory and the over read will give the correct value.

Changed 12 years ago by Erik Hovland <erik@…>

Moves AFCOffset to TSPacket so that we don't have funny indexing of _tsdata

Changed 12 years ago by Erik Hovland <erik@…>

init tspayload in ctor (so it does not have uninitialized values)

Changed 12 years ago by Erik Hovland <erik@…>

fix two spelling typos in the comments

comment:2 Changed 12 years ago by danielk

Resolution: fixed
Status: acceptedclosed

(In [17198]) Fixes #4918. Fixes type in comment.

Note on things that were not changed:

  • 'iff' is short for 'if and only if' -- not a typo of 'if'
  • TSHeader by itself is not a valid object, which is why we don't initialize the rest of the packet. I've added a comment.
  • AFCOffset was explained earlier in the ticket, I've added a short comment to the code.

comment:3 Changed 12 years ago by Erik Hovland <erik@…>

Thanks for following up. I appreciate it. It allows me to drop patching where possible (and unnecessary).

Note: See TracTickets for help on using tickets.