diff -Naurp pcp-6.2.0.orig/qa/1518 pcp-6.2.0/qa/1518 --- pcp-6.2.0.orig/qa/1518 1970-01-01 10:00:00.000000000 +1000 +++ pcp-6.2.0/qa/1518 2024-09-17 10:11:45.805874610 +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-6.2.0.orig/qa/1518.out pcp-6.2.0/qa/1518.out --- pcp-6.2.0.orig/qa/1518.out 1970-01-01 10:00:00.000000000 +1000 +++ pcp-6.2.0/qa/1518.out 2024-09-17 10:11:45.806874611 +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-6.2.0.orig/qa/group pcp-6.2.0/qa/group --- pcp-6.2.0.orig/qa/group 2024-02-11 23:48:09.000000000 +1100 +++ pcp-6.2.0/qa/group 2024-09-17 10:11:45.815874621 +1000 @@ -1951,6 +1951,7 @@ x11 1503 pcp pidstat ps python local 1511 pmcd local pmda.sample 1515 pmda.denki local valgrind +1518 pmcd libpcp local 1530 pmda.zfs local valgrind 1531 pmda.zfs local valgrind 1532 pmda.zfs local diff -Naurp pcp-6.2.0.orig/src/libpcp/src/endian.c pcp-6.2.0/src/libpcp/src/endian.c --- pcp-6.2.0.orig/src/libpcp/src/endian.c 2023-11-16 17:51:39.000000000 +1100 +++ pcp-6.2.0/src/libpcp/src/endian.c 2024-09-17 10:11:45.820874627 +1000 @@ -275,13 +275,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: @@ -305,6 +309,13 @@ __ntohpmValueBlock(pmValueBlock * const break; } } + +void +__ntohpmValueBlock(pmValueBlock * const vb) +{ + __ntohpmValueBlock_hdr(vb); + __ntohpmValueBlock_buf(vb); +} #endif #ifndef __htonpmPDUInfo diff -Naurp pcp-6.2.0.orig/src/libpcp/src/internal.h pcp-6.2.0/src/libpcp/src/internal.h --- pcp-6.2.0.orig/src/libpcp/src/internal.h 2023-11-16 17:51:39.000000000 +1100 +++ pcp-6.2.0/src/libpcp/src/internal.h 2024-09-17 10:11:45.823874630 +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 __htonf(a) /* noop */ #define __ntohf(a) /* noop */ #define __htond(a) /* noop */ @@ -90,6 +92,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 __htonf(char *) _PCP_HIDDEN; /* float */ #define __ntohf(v) __htonf(v) #define __htond(v) __htonll(v) /* double */ diff -Naurp pcp-6.2.0.orig/src/libpcp/src/p_result.c pcp-6.2.0/src/libpcp/src/p_result.c --- pcp-6.2.0.orig/src/libpcp/src/p_result.c 2023-11-16 17:51:39.000000000 +1100 +++ pcp-6.2.0/src/libpcp/src/p_result.c 2024-09-17 10:18:17.264314112 +1000 @@ -323,6 +323,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) int __pmDecodeValueSet(__pmPDU *pdubuf, int pdulen, __pmPDU *data, char *pduend, @@ -336,7 +465,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 */ @@ -414,11 +543,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]; @@ -427,29 +555,31 @@ __pmDecodeValueSet(__pmPDU *pdubuf, int check = (size_t)(pduend - (char *)pduvbp); if (sizeof(unsigned int) > check) { if (pmDebugOptions.pdu && pmDebugOptions.desperate) - fprintf(stderr, "%s: Bad: pmid[%d] value[%d] " - "second pduvp past end of " - "PDU buffer\n", - "__pmDecodeValueSet", i, j); + fprintf(stderr, "__pmDecodeValueSet: PM_ERR_IPC: pmid[%d] value[%d] second pduvp past end of PDU buffer\n", + 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) - fprintf(stderr, "%s: Bad: pmid[%d] value[%d] " - "vlen=%d\n", "__pmDecodeValueSet", - i, j, pduvbp->vlen); + if (pmDebugOptions.pdu) + fprintf(stderr, "__pmDecodeValueSet: PM_ERR_IPC: pmid[%d] value[%d] vlen=%d\n", + i, j, pduvbp->vlen); return PM_ERR_IPC; } - if (pduvbp->vlen > (size_t)(pduend - (char *)pduvbp)) { - if (pmDebugOptions.pdu && pmDebugOptions.desperate) - fprintf(stderr, "%s: Bad: pmid[%d] value[%d] " - "pduvp past end of PDU buffer\n", - "__pmDecodeValueSet", i, j); + if (pduvbp->vlen > check) { + if (pmDebugOptions.pdu) + 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", @@ -682,11 +812,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]; @@ -701,7 +830,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) @@ -710,13 +839,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; } }