pcp/0015-src-libpcp-src-p_result.c-hardening-of-result-PDU-ev.patch

From 5a7572c6e84dc5c7f42e170b90f275e96cfba77a Mon Sep 17 00:00:00 2001
From: Nathan Scott <nathans@redhat.com>
Date: Tue, 13 Aug 2024 06:54:25 +1000
Subject: [PATCH 15/19] src/libpcp/src/p_result.c: hardening of result PDU
 event record decoding

Updates to improve access to the result (store) PDU buffer when it
contains event records; adds buffer size overrun checking for each
component that has variable sizes encoded within the buffer.

Addresses SUSE Issue D.

(cherry picked from commit 1f45c69bac2b08613fda6e5438c25cf889b3224f)
[ddiss: rebase without 48ee02dc6 ("libpcp: extra diagnostics associated
 with PM_ERR_IPC")]
Acked-by: David Disseldorp <ddiss@suse.de>
---
 src/libpcp/src/endian.c   |  13 +++-
 src/libpcp/src/internal.h |   4 +
 src/libpcp/src/p_result.c | 153 +++++++++++++++++++++++++++++++++++---
 3 files changed, 158 insertions(+), 12 deletions(-)

diff --git a/src/libpcp/src/endian.c b/src/libpcp/src/endian.c
index 6dee5338a..fc6d931f9 100644
--- a/src/libpcp/src/endian.c
+++ b/src/libpcp/src/endian.c
@@ -275,13 +275,17 @@ ntohEventArray(pmValueBlock * const vb, int highres)
 }
 
 void
-__ntohpmValueBlock(pmValueBlock * const vb)
+__ntohpmValueBlock_hdr(pmValueBlock * const vb)
 {
     unsigned int	*ip = (unsigned int *)vb;
 
     /* Swab the first word, which contain vtype and vlen */
     *ip = ntohl(*ip);
+}
 
+void
+__ntohpmValueBlock_buf(pmValueBlock * const vb)
+{
     switch (vb->vtype) {
     case PM_TYPE_U64:
     case PM_TYPE_64:
@@ -305,6 +309,13 @@ __ntohpmValueBlock(pmValueBlock * const vb)
 	break;
     }
 }
+
+void
+__ntohpmValueBlock(pmValueBlock * const vb)
+{
+    __ntohpmValueBlock_hdr(vb);
+    __ntohpmValueBlock_buf(vb);
+}
 #endif
 
 #ifndef __htonpmPDUInfo
diff --git a/src/libpcp/src/internal.h b/src/libpcp/src/internal.h
index d0a2fd3c0..1e38f4796 100644
--- a/src/libpcp/src/internal.h
+++ b/src/libpcp/src/internal.h
@@ -60,6 +60,8 @@ extern int __pmGetDate(struct timespec *, char const *, struct timespec const *)
 #define __ntohpmLabel(a)	/* noop */
 #define __htonpmValueBlock(a)	/* noop */
 #define __ntohpmValueBlock(a)	/* noop */
+#define __ntohpmValueBlock_hdr(a)	/* noop */
+#define __ntohpmValueBlock_buf(a)	/* noop */
 #define __htonf(a)		/* noop */
 #define __ntohf(a)		/* noop */
 #define __htond(a)		/* noop */
@@ -90,6 +92,8 @@ extern void __htonpmLabel(pmLabel * const) _PCP_HIDDEN;
 extern void __ntohpmLabel(pmLabel * const) _PCP_HIDDEN;
 extern void __htonpmValueBlock(pmValueBlock * const) _PCP_HIDDEN;
 extern void __ntohpmValueBlock(pmValueBlock * const) _PCP_HIDDEN;
+extern void __ntohpmValueBlock_hdr(pmValueBlock * const) _PCP_HIDDEN;
+extern void __ntohpmValueBlock_buf(pmValueBlock * const) _PCP_HIDDEN;
 extern void __htonf(char *) _PCP_HIDDEN;	/* float */
 #define __ntohf(v) __htonf(v)
 #define __htond(v) __htonll(v)			/* double */
diff --git a/src/libpcp/src/p_result.c b/src/libpcp/src/p_result.c
index 071b52bba..b5a49df68 100644
--- a/src/libpcp/src/p_result.c
+++ b/src/libpcp/src/p_result.c
@@ -323,6 +323,124 @@ __pmSendHighResResult(int fd, int from, const __pmResult *result)
     return __pmSendHighResResult_ctx(NULL, fd, from, result);
 }
 
+/* Check that a network encoded event array is within a given buffer size */
+int
+__pmEventArrayCheck(pmValueBlock * const vb, int highres, int pmid, int value, size_t check)
+{
+    char		*base;
+    int			r;	/* records */
+    int			p;	/* parameters in a record ... */
+    int			nrecords;
+    int			nparams;
+
+    if (highres) {
+	pmHighResEventArray *hreap = (pmHighResEventArray *)vb;
+	base = (char *)&hreap->ea_record[0];
+	if (base > (char *)vb + check) {
+	    if (pmDebugOptions.pdu)
+		fprintf(stderr, "__pmEventArrayCheck: PM_ERR_IPC: pmid[%d] value[%d] highres event records past end of PDU buffer\n",
+			pmid, value);
+	    return PM_ERR_IPC;
+	}
+	nrecords = ntohl(hreap->ea_nrecords);
+    }
+    else {
+	pmEventArray *eap = (pmEventArray *)vb;
+	base = (char *)&eap->ea_record[0];
+	if (base > (char *)vb + check) {
+	    if (pmDebugOptions.pdu)
+		fprintf(stderr, "__pmEventArrayCheck: PM_ERR_IPC: pmid[%d] value[%d] event records past end of PDU buffer\n",
+			pmid, value);
+	    return PM_ERR_IPC;
+	}
+	nrecords = ntohl(eap->ea_nrecords);
+    }
+
+    /* walk packed event record array */
+    for (r = 0; r < nrecords; r++) {
+	unsigned int flags, type;
+	size_t size, remaining;
+
+	remaining = check - (base - (char *)vb);
+	if (highres) {
+	    pmHighResEventRecord *hrerp = (pmHighResEventRecord *)base;
+	    size = sizeof(hrerp->er_timestamp) + sizeof(hrerp->er_flags) +
+		    sizeof(hrerp->er_nparams);
+	    if (size > remaining) {
+		if (pmDebugOptions.pdu)
+		    fprintf(stderr, "__pmEventArrayCheck: PM_ERR_IPC: pmid[%d] value[%d] record[%d] highres event record past end of PDU buffer\n",
+			    pmid, value, r);
+		return PM_ERR_IPC;
+	    }
+	    nparams = ntohl(hrerp->er_nparams);
+	    flags = ntohl(hrerp->er_flags);
+	}
+	else {
+	    pmEventRecord *erp = (pmEventRecord *)base;
+	    size = sizeof(erp->er_timestamp) + sizeof(erp->er_flags) +
+		    sizeof(erp->er_nparams);
+	    if (size > remaining) {
+		if (pmDebugOptions.pdu)
+		    fprintf(stderr, "__pmEventArrayCheck: PM_ERR_IPC: pmid[%d] value[%d] record[%d] event record past end of PDU buffer\n",
+			    pmid, value, r);
+		return PM_ERR_IPC;
+	    }
+	    nparams = ntohl(erp->er_nparams);
+	    flags = ntohl(erp->er_flags);
+	}
+
+	if (flags & PM_EVENT_FLAG_MISSED)
+	    nparams = 0;
+
+	base += size;
+	remaining = check - (base - (char *)vb);
+
+	for (p = 0; p < nparams; p++) {
+	    __uint32_t		*tp;	/* points to int holding vtype/vlen */
+	    pmEventParameter	*epp = (pmEventParameter *)base;
+
+	    if (sizeof(pmEventParameter) > remaining) {
+		if (pmDebugOptions.pdu)
+		    fprintf(stderr, "__pmEventArrayCheck: PM_ERR_IPC: pmid[%d] value[%d] record[%d] param[%d] event record past end of PDU buffer\n",
+			    pmid, value, r, p);
+		return PM_ERR_IPC;
+	    }
+
+	    tp = (__uint32_t *)&epp->ep_pmid;
+	    tp++;		/* now points to ep_type/ep_len */
+	    *tp = ntohl(*tp);
+	    type = epp->ep_type;
+	    size = epp->ep_len;
+	    *tp = htonl(*tp);	/* leave the buffer how we found it */
+
+	    if (sizeof(pmID) + size > remaining) {
+		if (pmDebugOptions.pdu)
+		    fprintf(stderr, "__pmEventArrayCheck: PM_ERR_IPC: pmid[%d] value[%d] record[%d] param[%d] event record past end of PDU buffer\n",
+			    pmid, value, r, p);
+		return PM_ERR_IPC;
+	    }
+
+	    base += sizeof(pmID) + PM_PDU_SIZE_BYTES(size);
+
+	    size = 8;	/* 64-bit types */
+	    switch (type) {
+		case PM_TYPE_32:
+		case PM_TYPE_U32:
+		case PM_TYPE_FLOAT:
+		    size = 4;	/* 32-bit types */
+		    break;
+	    }
+	    if (sizeof(pmID) + size > remaining) {
+		if (pmDebugOptions.pdu)
+		    fprintf(stderr, "__pmEventArrayCheck: PM_ERR_IPC: pmid[%d] value[%d] record[%d] param[%d] event record past end of PDU buffer\n",
+			    pmid, value, r, p);
+		return PM_ERR_IPC;
+	    }
+	}
+    }
+    return 0;
+}
+
 #if defined(HAVE_64BIT_PTR)
 int
 __pmDecodeValueSet(__pmPDU *pdubuf, int pdulen, __pmPDU *data, char *pduend,
@@ -336,7 +454,7 @@ __pmDecodeValueSet(__pmPDU *pdubuf, int pdulen, __pmPDU *data, char *pduend,
     int		i, j;
 /*
  * Note: all sizes are in units of bytes ... beware that 'data' is in
- *	 units of __pmPDU
+ *	 units of __pmPDU (four bytes)
  */
     int		vsize;		/* size of vlist_t's in PDU buffer */
     int		nvsize;		/* size of pmValue's after decode */
@@ -433,7 +551,7 @@ __pmDecodeValueSet(__pmPDU *pdubuf, int pdulen, __pmPDU *data, char *pduend,
 			return PM_ERR_IPC;
 		    }
 
-		    __ntohpmValueBlock(pduvbp);
+		    __ntohpmValueBlock_hdr(pduvbp);
 		    if (pduvbp->vlen < PM_VAL_HDR_SIZE ||
 			pduvbp->vlen > pdulen) {
 			if (pmDebugOptions.pdu && pmDebugOptions.desperate)
@@ -442,13 +560,19 @@ __pmDecodeValueSet(__pmPDU *pdubuf, int pdulen, __pmPDU *data, char *pduend,
 					    i, j, pduvbp->vlen);
 			return PM_ERR_IPC;
 		    }
-		    if (pduvbp->vlen > (size_t)(pduend - (char *)pduvbp)) {
+		    if (pduvbp->vlen > check) {
 			if (pmDebugOptions.pdu && pmDebugOptions.desperate)
-			    fprintf(stderr, "%s: Bad: pmid[%d] value[%d] "
-					    "pduvp past end of PDU buffer\n",
-					    "__pmDecodeValueSet", i, j);
+			    fprintf(stderr, "__pmDecodeValueSet: PM_ERR_IPC: pmid[%d] value[%d] pduvp past end of PDU buffer\n",
+				i, j);
 			return PM_ERR_IPC;
 		    }
+		    if (pduvbp->vtype == PM_TYPE_HIGHRES_EVENT ||
+		        pduvbp->vtype == PM_TYPE_EVENT) {
+			vindex = (pduvbp->vtype == PM_TYPE_HIGHRES_EVENT);
+			if (__pmEventArrayCheck(pduvbp, vindex, i, j, check) < 0)
+			    return PM_ERR_IPC;
+		    }
+		    __ntohpmValueBlock_buf(pduvbp);
 		    vbsize += PM_PDU_SIZE_BYTES(pduvbp->vlen);
 		    if (pmDebugOptions.pdu && pmDebugOptions.desperate) {
 			fprintf(stderr, " len: %d type: %d",
@@ -700,7 +824,8 @@ __pmDecodeValueSet(__pmPDU *pdubuf, int pdulen, __pmPDU *data, char *pduend,
 					    "__pmDecodeValueSet", i, j);
 			return PM_ERR_IPC;
 		    }
-		    __ntohpmValueBlock(pduvbp);
+
+		    __ntohpmValueBlock_hdr(pduvbp);
 		    if (pduvbp->vlen < PM_VAL_HDR_SIZE ||
 			pduvbp->vlen > pdulen) {
 			if (pmDebugOptions.pdu && pmDebugOptions.desperate)
@@ -709,13 +834,19 @@ __pmDecodeValueSet(__pmPDU *pdubuf, int pdulen, __pmPDU *data, char *pduend,
 					    i, j, pduvbp->vlen);
 			return PM_ERR_IPC;
 		    }
-		    if (pduvbp->vlen > (size_t)(pduend - (char *)pduvbp)) {
+		    if (pduvbp->vlen > check) {
 			if (pmDebugOptions.pdu && pmDebugOptions.desperate)
-			    fprintf(stderr, "%s: Bad: pmid[%d] value[%d] "
-					    "pduvp past end of PDU buffer\n",
-					    "__pmDecodeValueSet", i, j);
+			    fprintf(stderr, "__pmDecodeValueSet: PM_ERR_IPC: pmid[%d] value[%d] pduvp past end of PDU buffer\n",
+				i, j);
 			return PM_ERR_IPC;
 		    }
+		    if (pduvbp->vtype == PM_TYPE_HIGHRES_EVENT ||
+		        pduvbp->vtype == PM_TYPE_EVENT) {
+			vindex = (pduvbp->vtype == PM_TYPE_HIGHRES_EVENT);
+			if (__pmEventArrayCheck(pduvbp, vindex, i, j, check) < 0)
+			    return PM_ERR_IPC;
+		    }
+		    __ntohpmValueBlock_buf(pduvbp);
 		    pduvp->value.pval = pduvbp;
 		}
 	    }
-- 
2.43.0