[PATCH] media: dvb_net: reject the pointer field at the cell boundary

[Guo Zihao]

dvb_net_ule_ts_pusi() checks the ULE payload pointer field before
skipping to the start of the first SNDU, but the comparison is off by
one:

        if (h->ts[4] > h->ts_remain) {
                pr_err("%lu: Invalid ULE packet (pointer field %d)\n",
                        h->priv->ts_count, h->ts[4]);
                ...
        }
        h->from_where = &h->ts[5] + h->ts[4];
        h->ts_remain -= 1 + h->ts[4];

ts_remain counts the bytes left in the cell after the four byte TS
header, so it starts at 184 and acts as an offset from &ts[4]. The
pointer field is an offset from &ts[5], one byte further along, so it
has to stay strictly below ts_remain. A value of exactly ts_remain
passes the test, and then

 - from_where points one byte past the end of the 188 byte cell, and
 - ts_remain -= 1 + ts[4] wraps around to 255, because ts_remain is u8.

The wrapped value is what turns this into more than a one byte overread.
It defeats the ts_remain < 2 check in dvb_net_ule_new_payload(), which
is the guard that stops the parser from reading the two byte SNDU length
out of bounds, and every later use of ts_remain in
dvb_net_ule_handle() works from the wrapped count.

Reject a pointer field of ts_remain as well. Values below it, including
the largest one that still leaves the length field readable, behave as
before.

No Fixes tag. The comparison and the from_where calculation next to it
were introduced together in 59142330aaea (2005) and have not been
touched since.

Reviewed-by: Fang Xieyan <fangxy@xxxxxxxxxxxx>
Signed-off-by: Guo Zihao <guozh23@xxxxxxxxxxxx>
---
The packet arrives through the demux, so a crafted MPEG2-TS stream
carrying a ULE SNDU with a pointer field of 184 reaches this code with
nothing else required to be well formed.

Pointer fields of 182 and 183 caused a separate endless loop in this
same block and were addressed in 29e1fa3565a7 ("dvb-core: Fix DoS bug in
ULE decapsulation code that can be triggered by an invalid Payload
Pointer"), which added the resync handling but left the comparison
operator alone.

handle_one_ule_extension() in this file got a bound of the same shape
last February (24d87712727a), so the ULE decapsulation path has taken
this kind of fix before.

 drivers/media/dvb-core/dvb_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-core/dvb_net.c b/drivers/media/dvb-core/dvb_net.c
index a2159b2bc..604c767e1 100644
--- a/drivers/media/dvb-core/dvb_net.c
+++ b/drivers/media/dvb-core/dvb_net.c
@@ -371,7 +371,7 @@ static int dvb_net_ule_ts_pusi(struct dvb_net_ule_handle *h)
 		/* Synchronize continuity counter. */
 		h->priv->tscc = h->ts[3] & 0x0F;
 		/* There is a pointer field here. */
-		if (h->ts[4] > h->ts_remain) {
+		if (h->ts[4] >= h->ts_remain) {
 			pr_err("%lu: Invalid ULE packet (pointer field %d)\n",
 				h->priv->ts_count, h->ts[4]);
 			h->ts += TS_SZ;
-- 
2.50.1