[PATCH] media: dst_ca: validate the CA message length in ca_set_pmt()

[Guo Zihao]

ca_set_pmt() takes the payload length from the ASN.1 length field of the
userspace supplied CA_PMT message and uses it to clear and fill
hw_buffer->msg, which is a fixed 256 byte array:

        length = asn_1_decode(&p_ca_message->msg[3]);
        debug_string(&p_ca_message->msg[4], length, 0);

        memset(hw_buffer->msg, '\0', length);
        handle_dst_tag(state, p_ca_message, hw_buffer, length);

asn_1_decode() implements the ASN.1 long form length, so msg[3] = 0x84
reads four length bytes and can return up to 0xffffffff. Nothing
compares the result against the size of the destination, so the memset()
overruns the kmalloc'd hw_buffer.

handle_dst_tag() does reject lengths above 247, but three things make it
ineffective here:

 - it runs after the memset() has already overflowed the buffer,
 - it is skipped entirely on the DST_TYPE_HAS_SESSION path,
 - debug_string() walks the message up to length and is called before
   any of the checks, so it reads out of bounds as well.

Validate the decoded length against sizeof(hw_buffer->msg) before it is
used, accounting for the tag that handle_dst_tag() prepends. Messages
that do not fit are rejected with -EINVAL.

No Fixes tag. asn_1_decode() and the ca_set_pmt() call chain come from
the original driver import, and the ASN.1 long form handling was last
touched in 93a14f15d35c (2005). The missing bound has been there since.

Reviewed-by: Liu Chao <liuc63@xxxxxxxxxxxx>
Signed-off-by: Guo Zihao <guozh23@xxxxxxxxxxxx>
---
Reaching this needs nothing more than a CA device node: open
/dev/dvb/adapterX/ca0 and issue CA_SEND_MSG with msg[0..2] set to
0x00 0x9f 0x80 (CA_PMT) and msg[3] = 0x84 followed by four length
bytes. No CAM has to be present, because the overrun happens before any
hardware access.

The 247 limit handle_dst_tag() uses is one byte short of what
sizeof(hw_buffer->msg) - tag_length allows, so the two bounds disagree.
Removing the check from handle_dst_tag() is not part of this patch, but
if a single check in one place is preferred, that would be the natural
follow-up.

cxd2099 bounds its CAM reply length the same way after a comparable
report (len > ecount || len < 2), so there is precedent in the DVB
frontend drivers.

 drivers/media/pci/bt8xx/dst_ca.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/media/pci/bt8xx/dst_ca.c b/drivers/media/pci/bt8xx/dst_ca.c
index 6da75c6ed..e126afc2f 100644
--- a/drivers/media/pci/bt8xx/dst_ca.c
+++ b/drivers/media/pci/bt8xx/dst_ca.c
@@ -401,6 +401,13 @@ static int ca_set_pmt(struct dst_state *state, struct ca_msg *p_ca_message, stru
 	u8 tag_length = 8;
 
 	length = asn_1_decode(&p_ca_message->msg[3]);
+	if (length > sizeof(hw_buffer->msg) - tag_length) {
+		dprintk(verbose, DST_CA_ERROR, 1,
+			" CA Message too long (%u) ! *** Bailing Out *** !",
+			length);
+		return -EINVAL;
+	}
+
 	dprintk(verbose, DST_CA_DEBUG, 1, " CA Message length=[%d]", length);
 	debug_string(&p_ca_message->msg[4], length, 0); /*	length is excluding tag & length	*/
 
-- 
2.50.1