diff -Naurp pcp-5.3.7.orig/qa/1518 pcp-5.3.7/qa/1518 --- pcp-5.3.7.orig/qa/1518 1970-01-01 10:00:00.000000000 +1000 +++ pcp-5.3.7/qa/1518 2024-09-09 13:13:14.561437414 +1000 @@ -0,0 +1,75 @@ +#!/bin/sh +# PCP QA Test No. 1518 +# SUSE Issue A) +# __pmDecodeValueSet() Miscalculates Available Buffer Space +# Leading to a Possible Heap Corruption +# +# Copyright (c) 2024 Ken McDonell. All Rights Reserved. +# Copyright (c) 2024 Matthias Gerstner. All Rights Reserved. +# + +if [ $# -eq 0 ] +then + seq=`basename $0` + echo "QA output created by $seq" +else + # use $seq from caller, unless not set + [ -n "$seq" ] || seq=`basename $0` + echo "QA output created by `basename $0` $*" +fi + +# get standard environment, filters and checks +. ./common.product +. ./common.filter +. ./common.check + +$sudo rm -rf $tmp $tmp.* $seq.full + +which nc >/dev/null 2>&1 || _notrun "no nc executable installed" +_check_valgrind + +_cleanup() +{ + cat pmcd.log >>$here/$seq.full + cd $here + $sudo rm -rf $tmp $tmp.* +} + +status=0 # success is the default! +trap "_cleanup; exit \$status" 0 1 2 3 15 + +_filter() +{ + sed \ + -e '/^Command: /d' \ + # end +} + +mkdir $tmp || exit 1 +cd $tmp +grep sampledso $PCP_PMCDCONF_PATH >pmcd.conf +cat pmcd.conf >>$here/$seq.full +port=`_find_free_port` +echo "port=$port" >>$here/$seq.full + +# real QA test starts here +valgrind $PCP_BINADM_DIR/pmcd -f -Dpdu -c ./pmcd.conf -s ./pmcd.socket -p $port >out 2>err & +valgrind_pid=$! +sleep 2 +pmcd_pid=`$PCP_PS_PROG $PCP_PS_ALL_FLAGS | grep '[p]mcd -f -Dpdu' | $PCP_AWK_PROG '{ print $2 }'` +echo "pmcd_pid=$pmcd_pid" >>$here/$seq.full +nc -N -U ./pmcd.socket <$here/binary/decode-value-set-out-of-bound-write 2>&1 \ +| od -c >>$here/$seq.full +sleep 2 +kill -TERM $pmcd_pid +wait + +echo "expect error to be logged ..." +grep __pmDecodeValueSet pmcd.log + +echo +echo "and no valgrind badness ..." +cat out err | _filter_valgrind | _filter + +# success, all done +exit diff -Naurp pcp-5.3.7.orig/qa/1518.out pcp-5.3.7/qa/1518.out --- pcp-5.3.7.orig/qa/1518.out 1970-01-01 10:00:00.000000000 +1000 +++ pcp-5.3.7/qa/1518.out 2024-09-09 13:13:14.561437414 +1000 @@ -0,0 +1,11 @@ +QA output created by 1518 +expect error to be logged ... +__pmDecodeValueSet: PM_ERR_IPC: pmid[0] value[0] vindex=1020 (max=255) + +and no valgrind badness ... +Memcheck, a memory error detector +LEAK SUMMARY: +definitely lost: 0 bytes in 0 blocks +indirectly lost: 0 bytes in 0 blocks +Rerun with --leak-check=full to see details of leaked memory +ERROR SUMMARY: 0 errors from 0 contexts ... diff -Naurp pcp-5.3.7.orig/qa/group pcp-5.3.7/qa/group --- pcp-5.3.7.orig/qa/group 2024-09-09 13:11:13.796450545 +1000 +++ pcp-5.3.7/qa/group 2024-09-09 13:13:49.419437747 +1000 @@ -1847,6 +1847,7 @@ x11 1490 python local labels 1495 pmlogrewrite labels pmdumplog local 1511 pmcd local pmda.sample +1518 pmcd libpcp local 1530 pmda.zfs local valgrind 1531 pmda.zfs local valgrind 1532 pmda.zfs local diff -Naurp pcp-5.3.7.orig/src/libpcp/src/endian.c pcp-5.3.7/src/libpcp/src/endian.c --- pcp-5.3.7.orig/src/libpcp/src/endian.c 2022-04-05 09:05:43.000000000 +1000 +++ pcp-5.3.7/src/libpcp/src/endian.c 2024-09-09 13:20:56.967560588 +1000 @@ -268,13 +268,17 @@ ntohEventArray(pmValueBlock * const vb, } 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: @@ -298,6 +302,13 @@ __ntohpmValueBlock(pmValueBlock * const break; } } + +void +__ntohpmValueBlock(pmValueBlock * const vb) +{ + __ntohpmValueBlock_hdr(vb); + __ntohpmValueBlock_buf(vb); +} #endif #ifndef __htonpmPDUInfo diff -Naurp pcp-5.3.7.orig/src/libpcp/src/internal.h pcp-5.3.7/src/libpcp/src/internal.h --- pcp-5.3.7.orig/src/libpcp/src/internal.h 2022-04-05 09:05:43.000000000 +1000 +++ pcp-5.3.7/src/libpcp/src/internal.h 2024-09-09 13:21:44.336608061 +1000 @@ -60,6 +60,8 @@ extern int __pmGetDate(struct timespec * #define __ntohpmLabel(a) /* noop */ #define __htonpmValueBlock(a) /* noop */ #define __ntohpmValueBlock(a) /* noop */ +#define __ntohpmValueBlock_hdr(a) /* noop */ +#define __ntohpmValueBlock_buf(a) /* noop */ #define __htonpmTimespec(a) /* noop */ #define __ntohpmTimespec(a) /* noop */ #define __htonpmTimestamp(a) /* noop */ @@ -94,6 +96,8 @@ extern void __htonpmLabel(pmLabel * cons 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 __htonpmTimespec(pmTimespec * const ) _PCP_HIDDEN; extern void __ntohpmTimespec(pmTimespec * const ) _PCP_HIDDEN; extern void __htonpmTimestamp(__pmTimestamp * const ); diff -Naurp pcp-5.3.7.orig/src/libpcp/src/p_result.c pcp-5.3.7/src/libpcp/src/p_result.c --- pcp-5.3.7.orig/src/libpcp/src/p_result.c 2022-04-05 09:05:43.000000000 +1000 +++ pcp-5.3.7/src/libpcp/src/p_result.c 2024-09-09 13:27:16.651969214 +1000 @@ -277,6 +277,135 @@ __pmSendHighResResult(int fd, int from, 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 #1: 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 #2: 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 #3: 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 #4: 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 #5: 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 #6: 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); + + /* + * final check for the types below, ep_len should be 4 or + * 8, but a malformed PDU could have smaller ep_len values + * and then unpacking these types risk going past the end + * of the PDU buffer + */ + size = 0; + switch (type) { + case PM_TYPE_32: + case PM_TYPE_U32: + case PM_TYPE_FLOAT: + size = 4; /* 32-bit types */ + break; + case PM_TYPE_64: + case PM_TYPE_U64: + case PM_TYPE_DOUBLE: + size = 8; /* 64-bit types */ + break; + } + if (size > 0 && sizeof(pmID) + size > remaining) { + if (pmDebugOptions.pdu) + fprintf(stderr, "__pmEventArrayCheck #7: 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) static int __pmDecodeValueSet(__pmPDU *pdubuf, int pdulen, __pmPDU *data, char *pduend, @@ -290,7 +419,7 @@ __pmDecodeValueSet(__pmPDU *pdubuf, int 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 */ @@ -368,11 +497,10 @@ __pmDecodeValueSet(__pmPDU *pdubuf, int return PM_ERR_IPC; } vindex = ntohl(pduvp->value.lval); - if (vindex < 0 || vindex > pdulen) { + if (vindex < 0 || (char *)&pdubuf[vindex] >= pduend) { if (pmDebugOptions.pdu && pmDebugOptions.desperate) - fprintf(stderr, "%s: Bad: pmid[%d] value[%d] " - "vindex=%d\n", - "__pmDecodeValueSet", i, j, vindex); + fprintf(stderr, "__pmDecodeValueSet: PM_ERR_IPC: pmid[%d] value[%d] vindex=%d (max=%ld)\n", + i, j, vindex, (long)((pduend-(char *)pdubuf) / sizeof(pdubuf[0])-1)); return PM_ERR_IPC; } pduvbp = (pmValueBlock *)&pdubuf[vindex]; @@ -387,7 +515,7 @@ __pmDecodeValueSet(__pmPDU *pdubuf, int "__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) @@ -396,13 +524,20 @@ __pmDecodeValueSet(__pmPDU *pdubuf, int 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); 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", @@ -635,11 +770,10 @@ __pmDecodeValueSet(__pmPDU *pdubuf, int } else { /* salvage pmValueBlocks from end of PDU */ vindex = ntohl(pduvp->value.lval); - if (vindex < 0 || vindex > pdulen) { + if (vindex < 0 || (char *)&pdubuf[vindex] >= pduend) { if (pmDebugOptions.pdu && pmDebugOptions.desperate) - fprintf(stderr, "%s: Bad: pmid[%d] value[%d] " - "vindex=%d\n", - "__pmDecodeValueSet", i, j, vindex); + fprintf(stderr, "__pmDecodeValueSet: PM_ERR_IPC: pmid[%d] value[%d] vindex=%d (max=%ld)\n", + i, j, vindex, (long)((pduend-(char *)pdubuf) / sizeof(pdubuf[0])-1)); return PM_ERR_IPC; } pduvbp = (pmValueBlock *)&pdubuf[vindex]; @@ -654,7 +788,8 @@ __pmDecodeValueSet(__pmPDU *pdubuf, int "__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) @@ -663,13 +798,20 @@ __pmDecodeValueSet(__pmPDU *pdubuf, int 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); 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; } }