[mythtv-users] recent mythbackend crashing

Douglas Paul doug at bogon.ca
Sat Jun 18 12:34:25 UTC 2022


On Sat, Jun 18, 2022 at 07:56:47AM +0200, Klaas de Waal wrote:
> It is not usual to do array boundary checking on this code level because of
> performance reasons; this code is executed for every 188 byte packet that
> is received and I leave the math for you how many packets per second this
> can be for 40 Mbit/s satellite transport stream.
> I will have a look at it.

This reminds me of a previous bug that I had encounted and fixed, but
did not follow up with fully because I was asked for and unable to
provide a TS capture to reproduce the problem (the station had fixed
their streams).

Unfortunately, in this flow, there are a number of places where the
received data, especially to do with offsets, number of segments, etc.,
is blindly trusted. I guess you could consider this a security issue,
depending how much you trust received TS packets over-the-air (it's
likely not a big issue).

There seems to be a lot of code already executed per received packet,
but it seems that that kind of bounds checking is still necessary when
you can't rule out the possibility of receiving incorrect data. It can
be mitigated somewhat by annotating the if statements with
unlikely/likely so the branch prediction is always correct. That can
really reduce the cost of checking for the normal case where everything
is fine. This is a lower rate than Ethernet packets on a 10Gb/s
connection (even a 1Gb/s connection ...) and we definitely want to check
received data in those cases for correctness.

I'll leave my commit here again, as I have kept it in my local branch
"just in case" but in this case it was due to the broadcaster putting
tables that should only be on cable networks, normally, containing
nothing but random garbage data. Probably there are other places where
similar things are done.

commit 43c15779592a1b8d9ebfc6a594cc263eb64f4a07
Author: Douglas Paul <doug at bogon.ca>
Date:   Wed Feb 20 12:56:32 2019 -0500

    Prevent buffer read overrun on corrupted tables

diff --git a/mythtv/libs/libmythtv/mpeg/sctetables.cpp b/mythtv/libs/libmythtv/mpeg/sctetables.cpp
index 6384fda32a..1c1f611e02 100644
--- a/mythtv/libs/libmythtv/mpeg/sctetables.cpp
+++ b/mythtv/libs/libmythtv/mpeg/sctetables.cpp
@@ -324,8 +324,13 @@ bool ShortVirtualChannelTable::Parse(void)
         }
         else
         {
-            for (uint i = 0; i < number_of_vc_records; i++)
+            for (uint i = 0; i < number_of_vc_records && ok; i++)
             {
+                if (next + 10 >= end)
+                {
+                    ok = false;
+                    break;
+                }
                 m_ptrs.push_back(next);
                 uint desc_count = next[10];
                 next += 10;


-- 
Douglas Paul




More information about the mythtv-users mailing list