File 0015-src-libpcp-src-p_result.c-hardening-of-result-PDU-ev.patch of Package pcp.34705
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
