diff --git a/pcp-7.1.5-OOB-pmDecodeInstance.patch b/pcp-7.1.5-OOB-pmDecodeInstance.patch new file mode 100644 index 0000000..dcfe5d5 --- /dev/null +++ b/pcp-7.1.5-OOB-pmDecodeInstance.patch @@ -0,0 +1,58 @@ +From b743fc5879 Mon Sep 17 00:00:00 2001 +From: Nathan Scott +Subject: [PATCH] libpcp: fix OOB read in __pmDecodeInstance (CWE-125/195) + +The __pmDecodeInstance() loop advances ip by the PDU alignment-padded +entry size after each instance. When namelen % 4 != 0, the padding +advance can push ip past pdu_end. The existing bounds check casts the +pointer difference to size_t: (size_t)(pdu_end - (char *)ip). When ip +is past pdu_end, this produces a negative ptrdiff_t that wraps to a +very large size_t, causing both bounds checks to silently pass. +Execution falls through to memcpy reading past the PDU buffer. + +Fix: add an explicit signed pointer guard at the top of each loop +iteration — if ((char *)ip >= pdu_end) — before the size_t cast. +This ensures the subsequent unsigned comparison is always valid. + +Reported-by: Francisco Alisson Bezerra, TIM Security Red Team +Reported-by: Lucas Gabriel Alves, TIM Security Red Team +Reported-by: Massimiliano Brolli, TIM Security Red Team + +Co-Authored-By: Claude Opus 4.6 (1M context) +--- +diff --git a/src/libpcp/src/p_instance.c b/src/libpcp/src/p_instance.c +index de05972f14..8c3c125c48 100644 +--- a/src/libpcp/src/p_instance.c ++++ b/src/libpcp/src/p_instance.c +@@ -289,6 +289,13 @@ __pmDecodeInstance(__pmPDU *pdubuf, pmInResult **result) + pdu_used = (char *)&pp->rest[0]; + for (i = j = 0; i < res->numinst; i++) { + ip = (instlist_t *)&pp->rest[j/sizeof(__pmPDU)]; ++ if ((char *)ip >= pdu_end) { ++ if (pmDebugOptions.pdu) ++ fprintf(stderr, "%s: PM_ERR_IPC: inst[%d] ip past pdu_end\n", ++ __FUNCTION__, i); ++ sts = PM_ERR_IPC; ++ goto badsts; ++ } + if (sizeof(instlist_t) - sizeof(ip->name) > (size_t)(pdu_end - (char *)ip)) { + if (pmDebugOptions.pdu) { + fprintf(stderr, "__pmDecodeInstance: PM_ERR_IPC: sizeof(instlist_t) %d - sizeof(name) %d > remainder %d\n", +diff --git a/src/libpcp3/src/p_instance.c b/src/libpcp3/src/p_instance.c +index 6881ff37ef..7d836e75a0 100644 +--- a/src/libpcp3/src/p_instance.c ++++ b/src/libpcp3/src/p_instance.c +@@ -290,6 +290,13 @@ __pmDecodeInstance(__pmPDU *pdubuf, pmInResult **result) + pdu_used = (char *)&pp->rest[0]; + for (i = j = 0; i < res->numinst; i++) { + ip = (instlist_t *)&pp->rest[j/sizeof(__pmPDU)]; ++ if ((char *)ip >= pdu_end) { ++ if (pmDebugOptions.pdu) ++ fprintf(stderr, "%s: PM_ERR_IPC: inst[%d] ip past pdu_end\n", ++ __FUNCTION__, i); ++ sts = PM_ERR_IPC; ++ goto badsts; ++ } + if (sizeof(instlist_t) - sizeof(ip->name) > (size_t)(pdu_end - (char *)ip)) { + if (pmDebugOptions.pdu) { + fprintf(stderr, "__pmDecodeInstance: PM_ERR_IPC: sizeof(instlist_t) %d - sizeof(name) %d > remainder %d\n", diff --git a/pcp-7.1.5-OOB-pmDecodeLabel.patch b/pcp-7.1.5-OOB-pmDecodeLabel.patch new file mode 100644 index 0000000..af53139 --- /dev/null +++ b/pcp-7.1.5-OOB-pmDecodeLabel.patch @@ -0,0 +1,57 @@ +From e512482e7d Mon Sep 17 00:00:00 2001 +From: Nathan Scott +Subject: [PATCH] libpcp: fix OOB read in __pmDecodeLabel via negative jsonoff (CWE-125) + +The bounds check 'if (pdu_length < jsonoff + jsonlen)' uses signed +arithmetic. When jsonoff is negative (high bit set after ntohl) and +jsonlen is a small positive value, their sum wraps to a small positive +number, passing the check. The subsequent memcpy reads from +label_pdu + jsonoff, an address before the start of the PDU buffer. + +Fix: reject negative jsonoff and jsonlen explicitly, then use unsigned +(size_t) arithmetic for the bounds check to prevent signed wraparound. + +Reported-by: Francisco Alisson Bezerra, TIM Security Red Team +Reported-by: Lucas Gabriel Alves, TIM Security Red Team +Reported-by: Massimiliano Brolli, TIM Security Red Team + +Co-Authored-By: Claude Opus 4.6 (1M context) +--- +diff --git a/src/libpcp/src/p_label.c b/src/libpcp/src/p_label.c +index 10a348c8f0..95dbc86455 100644 +--- a/src/libpcp/src/p_label.c ++++ b/src/libpcp/src/p_label.c +@@ -446,10 +446,11 @@ __pmDecodeLabel(__pmPDU *pdubuf, int *ident, int *type, pmLabelSet **setsp, int + } + + /* check JSON content fits within the PDU bounds */ +- if (pdu_length < jsonoff + jsonlen) { ++ if (jsonoff < 0 || jsonlen < 0 || ++ (size_t)jsonoff + (size_t)jsonlen > pdu_length) { + if (pmDebugOptions.pdu) { +- fprintf(stderr, "__pmDecodeLabel: PM_ERR_IPC: labelset[%d] pdu_length %d < jsonoff %d + jsonlen %d\n", +- i, (int)pdu_length, jsonoff, jsonlen); ++ fprintf(stderr, "%s: PM_ERR_IPC: labelset[%d] pdu_length %d < jsonoff %d + jsonlen %d\n", ++ __FUNCTION__, i, (int)pdu_length, jsonoff, jsonlen); + } + goto corrupt; + } +diff --git a/src/libpcp3/src/p_label.c b/src/libpcp3/src/p_label.c +index 10a348c8f0..95dbc86455 100644 +--- a/src/libpcp3/src/p_label.c ++++ b/src/libpcp3/src/p_label.c +@@ -446,10 +446,11 @@ __pmDecodeLabel(__pmPDU *pdubuf, int *ident, int *type, pmLabelSet **setsp, int + } + + /* check JSON content fits within the PDU bounds */ +- if (pdu_length < jsonoff + jsonlen) { ++ if (jsonoff < 0 || jsonlen < 0 || ++ (size_t)jsonoff + (size_t)jsonlen > pdu_length) { + if (pmDebugOptions.pdu) { +- fprintf(stderr, "__pmDecodeLabel: PM_ERR_IPC: labelset[%d] pdu_length %d < jsonoff %d + jsonlen %d\n", +- i, (int)pdu_length, jsonoff, jsonlen); ++ fprintf(stderr, "%s: PM_ERR_IPC: labelset[%d] pdu_length %d < jsonoff %d + jsonlen %d\n", ++ __FUNCTION__, i, (int)pdu_length, jsonoff, jsonlen); + } + goto corrupt; + } diff --git a/pcp-7.1.5-OOB-pmDecodeLogStatus.patch b/pcp-7.1.5-OOB-pmDecodeLogStatus.patch new file mode 100644 index 0000000..fa0e674 --- /dev/null +++ b/pcp-7.1.5-OOB-pmDecodeLogStatus.patch @@ -0,0 +1,472 @@ +From 5366a546d6 Mon Sep 17 00:00:00 2001 +From: Nathan Scott +Subject: [PATCH] libpcp: fix OOB read in __pmDecodeLogStatus (CWE-125) + +For each of the six length-prefixed string fields in PDU_LOG_STATUS +(hostname, fqdn, timezone, zoneinfo for both pmcd and pmlogger), +strdup(p) was called before verifying that p+len falls within the +PDU buffer. strdup reads until a null byte, so a non-null-terminated +string causes reads past the PDU boundary into adjacent heap memory. + +Fix: for all six fields, move the p+len > pduend bounds check before +the string copy, and replace strdup(p) with strndup(p, len) to +respect the declared length regardless of null terminator presence. + +Reported-by: Francisco Alisson Bezerra, TIM Security Red Team +Reported-by: Lucas Gabriel Alves, TIM Security Red Team +Reported-by: Massimiliano Brolli, TIM Security Red Team + +Co-Authored-By: Claude Opus 4.6 (1M context) +--- +diff --git a/src/libpcp/src/p_lstatus.c b/src/libpcp/src/p_lstatus.c +index 8cc833e3ea..874d0bc52c 100644 +--- a/src/libpcp/src/p_lstatus.c ++++ b/src/libpcp/src/p_lstatus.c +@@ -265,157 +265,157 @@ __pmDecodeLogStatus(__pmPDU *pdubuf, __pmLoggerStatus **result) + if (len == 0) + lsp->pmcd.hostname = NULL; + else { +- if (len > PM_MAX_HOSTNAMELEN) { +- /* cannot be longer than hostname in archive label */ ++ if (len < 0 || len > PM_MAX_HOSTNAMELEN) { ++ /* cannot be negative or longer than hostname in archive label */ + if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmcd.hostname too long (%d)\n", len); ++ fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: invalid pmcd.hostname (%d)\n", len); + __pmFreeLogStatus(lsp, 1); + return PM_ERR_IPC; + } +- if ((lsp->pmcd.hostname = strdup(p)) == NULL) { ++ if (p + len > pduend) { ++ if (pmDebugOptions.pmlc || pmDebugOptions.pdu) ++ fprintf(stderr, "%s: PM_ERR_IPC: pmcd.hostname data[%ld] > PDU len (%d)\n", ++ __FUNCTION__, (long)(p + len - (char *)&pp->data[0]), pp->hdr.len); ++ __pmFreeLogStatus(lsp, 1); ++ return PM_ERR_IPC; ++ } ++ if ((lsp->pmcd.hostname = strndup(p, len)) == NULL) { + sts = -oserror(); + pmNoMem("__pmDecodeLogStatus: pmcd.hostname", len, PM_RECOV_ERR); + __pmFreeLogStatus(lsp, 1); + return sts; + } + p += len; +- if (p > pduend) { +- if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmcd.hostname data[%ld] > PDU len (%d)\n", +- (long)(p - (char *)&pp->data[0]), pp->hdr.len); +- __pmFreeLogStatus(lsp, 1); +- return PM_ERR_IPC; +- } + } + len = ntohl(pp->pmcd_fqdn_len); + if (len == 0) + lsp->pmcd.fqdn = NULL; + else { +- if (len > PM_MAX_HOSTNAMELEN) { +- /* cannot be longer than hostname in archive label */ ++ if (len < 0 || len > PM_MAX_HOSTNAMELEN) { ++ /* cannot be negative or longer than hostname in archive label */ + if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmcd.fqdn too long (%d)\n", len); ++ fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: invalid pmcd.fqdn (%d)\n", len); + __pmFreeLogStatus(lsp, 1); + return PM_ERR_IPC; + } +- if ((lsp->pmcd.fqdn = strdup(p)) == NULL) { ++ if (p + len > pduend) { ++ if (pmDebugOptions.pmlc || pmDebugOptions.pdu) ++ fprintf(stderr, "%s: PM_ERR_IPC: pmcd.fqdn data[%ld] > PDU len (%d)\n", ++ __FUNCTION__, (long)(p + len - (char *)&pp->data[0]), pp->hdr.len); ++ __pmFreeLogStatus(lsp, 1); ++ return PM_ERR_IPC; ++ } ++ if ((lsp->pmcd.fqdn = strndup(p, len)) == NULL) { + sts = -oserror(); + pmNoMem("__pmDecodeLogStatus: pmcd.fqdn", len, PM_RECOV_ERR); + __pmFreeLogStatus(lsp, 1); + return sts; + } + p += len; +- if (p > pduend) { +- if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmcd.fqdn data[%ld] > PDU len (%d)\n", +- (long)(p - (char *)&pp->data[0]), pp->hdr.len); +- __pmFreeLogStatus(lsp, 1); +- return PM_ERR_IPC; +- } + } + len = ntohl(pp->pmcd_timezone_len); + if (len == 0) + lsp->pmcd.timezone = NULL; + else { +- if (len > PM_MAX_TIMEZONELEN) { +- /* cannot be longer than timezone in archive label */ ++ if (len < 0 || len > PM_MAX_TIMEZONELEN) { ++ /* cannot be negative or longer than timezone in archive label */ + if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmcd.timezone too long (%d)\n", len); ++ fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: invalid pmcd.timezone (%d)\n", len); + __pmFreeLogStatus(lsp, 1); + return PM_ERR_IPC; + } +- if ((lsp->pmcd.timezone = strdup(p)) == NULL) { ++ if (p + len > pduend) { ++ if (pmDebugOptions.pmlc || pmDebugOptions.pdu) ++ fprintf(stderr, "%s: PM_ERR_IPC: pmcd.timezone data[%ld] > PDU len (%d)\n", ++ __FUNCTION__, (long)(p + len - (char *)&pp->data[0]), pp->hdr.len); ++ __pmFreeLogStatus(lsp, 1); ++ return PM_ERR_IPC; ++ } ++ if ((lsp->pmcd.timezone = strndup(p, len)) == NULL) { + sts = -oserror(); + pmNoMem("__pmDecodeLogStatus: pmcd.timezone", len, PM_RECOV_ERR); + __pmFreeLogStatus(lsp, 1); + return sts; + } + p += len; +- if (p > pduend) { +- if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmcd.timezone data[%ld] > PDU len (%d)\n", +- (long)(p - (char *)&pp->data[0]), pp->hdr.len); +- __pmFreeLogStatus(lsp, 1); +- return PM_ERR_IPC; +- } + } + len = ntohl(pp->pmcd_zoneinfo_len); + if (len == 0) + lsp->pmcd.zoneinfo = NULL; + else { +- if (len > PM_MAX_ZONEINFOLEN) { +- /* cannot be longer than zoneinfo in archive label */ ++ if (len < 0 || len > PM_MAX_ZONEINFOLEN) { ++ /* cannot be negative or longer than zoneinfo in archive label */ + if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmcd.zoneinfo too long (%d)\n", len); ++ fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: invalid pmcd.zoneinfo (%d)\n", len); + __pmFreeLogStatus(lsp, 1); + return PM_ERR_IPC; + } +- if ((lsp->pmcd.zoneinfo = strdup(p)) == NULL) { ++ if (p + len > pduend) { ++ if (pmDebugOptions.pmlc || pmDebugOptions.pdu) ++ fprintf(stderr, "%s: PM_ERR_IPC: pmcd.zoneinfo data[%ld] > PDU len (%d)\n", ++ __FUNCTION__, (long)(p + len - (char *)&pp->data[0]), pp->hdr.len); ++ __pmFreeLogStatus(lsp, 1); ++ return PM_ERR_IPC; ++ } ++ if ((lsp->pmcd.zoneinfo = strndup(p, len)) == NULL) { + sts = -oserror(); + pmNoMem("__pmDecodeLogStatus: pmcd.zoneinfo", len, PM_RECOV_ERR); + __pmFreeLogStatus(lsp, 1); + return sts; + } + p += len; +- if (p > pduend) { +- if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmcd.zoneinfo data[%ld] > PDU len (%d)\n", +- (long)(p - (char *)&pp->data[0]), pp->hdr.len); +- __pmFreeLogStatus(lsp, 1); +- return PM_ERR_IPC; +- } + } + len = ntohl(pp->pmlogger_timezone_len); + if (len == 0) + lsp->pmlogger.timezone = NULL; + else { + if (len > PM_MAX_TIMEZONELEN) { +- /* cannot be longer than timezone in archive label */ ++ /* cannot be negative or longer than timezone in archive label */ + if (pmDebugOptions.pmlc || pmDebugOptions.pdu) + fprintf(stderr, "__pmDecodeLogStatusPM_ERR_IPC: : pmlogger.timezone too long (%d)\n", len); + __pmFreeLogStatus(lsp, 1); + return PM_ERR_IPC; + } +- if ((lsp->pmlogger.timezone = strdup(p)) == NULL) { ++ if (p + len > pduend) { ++ if (pmDebugOptions.pmlc || pmDebugOptions.pdu) ++ fprintf(stderr, "%s: PM_ERR_IPC: pmlogger.timezone data[%ld] > PDU len (%d)\n", ++ __FUNCTION__, (long)(p + len - (char *)&pp->data[0]), pp->hdr.len); ++ __pmFreeLogStatus(lsp, 1); ++ return PM_ERR_IPC; ++ } ++ if ((lsp->pmlogger.timezone = strndup(p, len)) == NULL) { + sts = -oserror(); + pmNoMem("__pmDecodeLogStatus: pmlogger.timezone", len, PM_RECOV_ERR); + __pmFreeLogStatus(lsp, 1); + return sts; + } + p += len; +- if (p > pduend) { +- if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmlogger.timezone data[%ld] > PDU len (%d)\n", +- (long)(p - (char *)&pp->data[0]), pp->hdr.len); +- __pmFreeLogStatus(lsp, 1); +- return PM_ERR_IPC; +- } + } + len = ntohl(pp->pmlogger_zoneinfo_len); + if (len == 0) + lsp->pmlogger.zoneinfo = NULL; + else { +- if (len > PM_MAX_ZONEINFOLEN) { +- /* cannot be longer than zoneinfo in archive label */ ++ if (len < 0 || len > PM_MAX_ZONEINFOLEN) { ++ /* cannot be negative or longer than zoneinfo in archive label */ + if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmlogger.zoneinfo too long (%d)\n", len); ++ fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: invalid pmlogger.zoneinfo (%d)\n", len); ++ __pmFreeLogStatus(lsp, 1); ++ return PM_ERR_IPC; ++ } ++ if (p + len > pduend) { ++ if (pmDebugOptions.pmlc || pmDebugOptions.pdu) ++ fprintf(stderr, "%s: PM_ERR_IPC: pmlogger.zoneinfo data[%ld] > PDU len (%d)\n", ++ __FUNCTION__, (long)(p + len - (char *)&pp->data[0]), pp->hdr.len); + __pmFreeLogStatus(lsp, 1); + return PM_ERR_IPC; + } +- if ((lsp->pmlogger.zoneinfo = strdup(p)) == NULL) { ++ if ((lsp->pmlogger.zoneinfo = strndup(p, len)) == NULL) { + sts = -oserror(); + pmNoMem("__pmDecodeLogStatus: pmlogger.zoneinfo", len, PM_RECOV_ERR); + __pmFreeLogStatus(lsp, 1); + return sts; + } + p += len; +- if (p > pduend) { +- if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmlogger.zoneinfo data[%ld] > PDU len (%d)\n", +- (long)(p - (char *)&pp->data[0]), pp->hdr.len); +- __pmFreeLogStatus(lsp, 1); +- return PM_ERR_IPC; +- } + } + } + else if (version == LOG_PDU_VERSION2) { +diff --git a/src/libpcp3/src/p_lstatus.c b/src/libpcp3/src/p_lstatus.c +index d9053b5d1a..2bc4037b13 100644 +--- a/src/libpcp3/src/p_lstatus.c ++++ b/src/libpcp3/src/p_lstatus.c +@@ -265,157 +265,154 @@ __pmDecodeLogStatus(__pmPDU *pdubuf, __pmLoggerStatus **result) + if (len == 0) + lsp->pmcd.hostname = NULL; + else { +- if (len > PM_MAX_HOSTNAMELEN) { +- /* cannot be longer than hostname in archive label */ ++ if (len < 0 || len > PM_MAX_HOSTNAMELEN) { ++ /* cannot be negative or longer than hostname in archive label */ + if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmcd.hostname too long (%d)\n", len); ++ fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: invalid pmcd.hostname (%d)\n", len); ++ __pmFreeLogStatus(lsp, 1); ++ return PM_ERR_IPC; ++ } ++ if (p + len > pduend) { ++ if (pmDebugOptions.pmlc || pmDebugOptions.pdu) ++ fprintf(stderr, "%s: PM_ERR_IPC: pmcd.hostname data[%ld] > PDU len (%d)\n", ++ __FUNCTION__, (long)(p + len - (char *)&pp->data[0]), pp->hdr.len); + __pmFreeLogStatus(lsp, 1); + return PM_ERR_IPC; + } +- if ((lsp->pmcd.hostname = strdup(p)) == NULL) { ++ if ((lsp->pmcd.hostname = strndup(p, len)) == NULL) { + sts = -oserror(); + pmNoMem("__pmDecodeLogStatus: pmcd.hostname", len, PM_RECOV_ERR); + __pmFreeLogStatus(lsp, 1); + return sts; + } + p += len; +- if (p > pduend) { +- if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmcd.hostname data[%ld] > PDU len (%d)\n", +- (long)(p - (char *)&pp->data[0]), pp->hdr.len); +- __pmFreeLogStatus(lsp, 1); +- return PM_ERR_IPC; +- } + } + len = ntohl(pp->pmcd_fqdn_len); + if (len == 0) + lsp->pmcd.fqdn = NULL; + else { + if (len > PM_MAX_HOSTNAMELEN) { +- /* cannot be longer than hostname in archive label */ +- if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmcd.fqdn too long (%d)\n", len); ++ if (pmDebugOptions.pmlc || pmDebugOptions.pdu) ++ fprintf(stderr, "%s: PM_ERR_IPC: pmcd.fqdn too long (%d)\n", __FUNCTION__, len); ++ __pmFreeLogStatus(lsp, 1); ++ return PM_ERR_IPC; ++ } ++ if (p + len > pduend) { ++ if (pmDebugOptions.pmlc || pmDebugOptions.pdu) ++ fprintf(stderr, "%s: PM_ERR_IPC: pmcd.fqdn data[%ld] > PDU len (%d)\n", ++ __FUNCTION__, (long)(p + len - (char *)&pp->data[0]), pp->hdr.len); + __pmFreeLogStatus(lsp, 1); + return PM_ERR_IPC; + } +- if ((lsp->pmcd.fqdn = strdup(p)) == NULL) { ++ if ((lsp->pmcd.fqdn = strndup(p, len)) == NULL) { + sts = -oserror(); + pmNoMem("__pmDecodeLogStatus: pmcd.fqdn", len, PM_RECOV_ERR); + __pmFreeLogStatus(lsp, 1); + return sts; + } + p += len; +- if (p > pduend) { +- if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmcd.fqdn data[%ld] > PDU len (%d)\n", +- (long)(p - (char *)&pp->data[0]), pp->hdr.len); +- __pmFreeLogStatus(lsp, 1); +- return PM_ERR_IPC; +- } + } + len = ntohl(pp->pmcd_timezone_len); + if (len == 0) + lsp->pmcd.timezone = NULL; + else { + if (len > PM_MAX_TIMEZONELEN) { +- /* cannot be longer than timezone in archive label */ +- if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmcd.timezone too long (%d)\n", len); ++ if (pmDebugOptions.pmlc || pmDebugOptions.pdu) ++ fprintf(stderr, "%s: PM_ERR_IPC: pmcd.timezone too long (%d)\n", __FUNCTION__, len); ++ __pmFreeLogStatus(lsp, 1); ++ return PM_ERR_IPC; ++ } ++ if (p + len > pduend) { ++ if (pmDebugOptions.pmlc || pmDebugOptions.pdu) ++ fprintf(stderr, "%s: PM_ERR_IPC: pmcd.timezone data[%ld] > PDU len (%d)\n", ++ __FUNCTION__, (long)(p + len - (char *)&pp->data[0]), pp->hdr.len); + __pmFreeLogStatus(lsp, 1); + return PM_ERR_IPC; + } +- if ((lsp->pmcd.timezone = strdup(p)) == NULL) { ++ if ((lsp->pmcd.timezone = strndup(p, len)) == NULL) { + sts = -oserror(); + pmNoMem("__pmDecodeLogStatus: pmcd.timezone", len, PM_RECOV_ERR); + __pmFreeLogStatus(lsp, 1); + return sts; + } + p += len; +- if (p > pduend) { +- if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmcd.timezone data[%ld] > PDU len (%d)\n", +- (long)(p - (char *)&pp->data[0]), pp->hdr.len); +- __pmFreeLogStatus(lsp, 1); +- return PM_ERR_IPC; +- } + } + len = ntohl(pp->pmcd_zoneinfo_len); + if (len == 0) + lsp->pmcd.zoneinfo = NULL; + else { + if (len > PM_MAX_ZONEINFOLEN) { +- /* cannot be longer than zoneinfo in archive label */ +- if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmcd.zoneinfo too long (%d)\n", len); ++ if (pmDebugOptions.pmlc || pmDebugOptions.pdu) ++ fprintf(stderr, "%s: PM_ERR_IPC: pmcd.zoneinfo too long (%d)\n", __FUNCTION__, len); + __pmFreeLogStatus(lsp, 1); + return PM_ERR_IPC; + } +- if ((lsp->pmcd.zoneinfo = strdup(p)) == NULL) { ++ if (p + len > pduend) { ++ if (pmDebugOptions.pmlc || pmDebugOptions.pdu) ++ fprintf(stderr, "%s: PM_ERR_IPC: pmcd.zoneinfo data[%ld] > PDU len (%d)\n", ++ __FUNCTION__, (long)(p + len - (char *)&pp->data[0]), pp->hdr.len); ++ __pmFreeLogStatus(lsp, 1); ++ return PM_ERR_IPC; ++ } ++ if ((lsp->pmcd.zoneinfo = strndup(p, len)) == NULL) { + sts = -oserror(); + pmNoMem("__pmDecodeLogStatus: pmcd.zoneinfo", len, PM_RECOV_ERR); + __pmFreeLogStatus(lsp, 1); + return sts; + } + p += len; +- if (p > pduend) { +- if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmcd.zoneinfo data[%ld] > PDU len (%d)\n", +- (long)(p - (char *)&pp->data[0]), pp->hdr.len); +- __pmFreeLogStatus(lsp, 1); +- return PM_ERR_IPC; +- } + } + len = ntohl(pp->pmlogger_timezone_len); + if (len == 0) + lsp->pmlogger.timezone = NULL; + else { +- if (len > PM_MAX_TIMEZONELEN) { +- /* cannot be longer than timezone in archive label */ ++ if (len < 0 || len > PM_MAX_TIMEZONELEN) { ++ /* cannot be negative or longer than timezone in archive label */ + if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatusPM_ERR_IPC: : pmlogger.timezone too long (%d)\n", len); ++ fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: invalid pmlogger.timezone (%d)\n", len); ++ __pmFreeLogStatus(lsp, 1); ++ return PM_ERR_IPC; ++ } ++ if (p + len > pduend) { ++ if (pmDebugOptions.pmlc || pmDebugOptions.pdu) ++ fprintf(stderr, "%s: PM_ERR_IPC: pmlogger.timezone data[%ld] > PDU len (%d)\n", ++ __FUNCTION__, (long)(p + len - (char *)&pp->data[0]), pp->hdr.len); + __pmFreeLogStatus(lsp, 1); + return PM_ERR_IPC; + } +- if ((lsp->pmlogger.timezone = strdup(p)) == NULL) { ++ if ((lsp->pmlogger.timezone = strndup(p, len)) == NULL) { + sts = -oserror(); + pmNoMem("__pmDecodeLogStatus: pmlogger.timezone", len, PM_RECOV_ERR); + __pmFreeLogStatus(lsp, 1); + return sts; + } + p += len; +- if (p > pduend) { +- if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmlogger.timezone data[%ld] > PDU len (%d)\n", +- (long)(p - (char *)&pp->data[0]), pp->hdr.len); +- __pmFreeLogStatus(lsp, 1); +- return PM_ERR_IPC; +- } + } + len = ntohl(pp->pmlogger_zoneinfo_len); + if (len == 0) + lsp->pmlogger.zoneinfo = NULL; + else { +- if (len > PM_MAX_ZONEINFOLEN) { +- /* cannot be longer than zoneinfo in archive label */ +- if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmlogger.zoneinfo too long (%d)\n", len); ++ if (len < 0 || len > PM_MAX_ZONEINFOLEN) { ++ /* cannot be negative or longer than zoneinfo in archive label */ ++ if (pmDebugOptions.pmlc || pmDebugOptions.pdu) ++ fprintf(stderr, "%s: PM_ERR_IPC: invalid pmlogger.zoneinfo (%d)\n", __FUNCTION__, len); ++ __pmFreeLogStatus(lsp, 1); ++ return PM_ERR_IPC; ++ } ++ if (p + len > pduend) { ++ if (pmDebugOptions.pmlc || pmDebugOptions.pdu) ++ fprintf(stderr, "%s: PM_ERR_IPC: pmlogger.zoneinfo data[%ld] > PDU len (%d)\n", ++ __FUNCTION__, (long)(p + len - (char *)&pp->data[0]), pp->hdr.len); + __pmFreeLogStatus(lsp, 1); + return PM_ERR_IPC; + } +- if ((lsp->pmlogger.zoneinfo = strdup(p)) == NULL) { ++ if ((lsp->pmlogger.zoneinfo = strndup(p, len)) == NULL) { + sts = -oserror(); + pmNoMem("__pmDecodeLogStatus: pmlogger.zoneinfo", len, PM_RECOV_ERR); + __pmFreeLogStatus(lsp, 1); + return sts; + } + p += len; +- if (p > pduend) { +- if (pmDebugOptions.pmlc || pmDebugOptions.pdu) +- fprintf(stderr, "__pmDecodeLogStatus: PM_ERR_IPC: pmlogger.zoneinfo data[%ld] > PDU len (%d)\n", +- (long)(p - (char *)&pp->data[0]), pp->hdr.len); +- __pmFreeLogStatus(lsp, 1); +- return PM_ERR_IPC; +- } + } + } + else if (version == LOG_PDU_VERSION2) { diff --git a/pcp-7.1.5-OOB-pmDiscoverDecodeMetaInDom.patch b/pcp-7.1.5-OOB-pmDiscoverDecodeMetaInDom.patch new file mode 100644 index 0000000..ca84857 --- /dev/null +++ b/pcp-7.1.5-OOB-pmDiscoverDecodeMetaInDom.patch @@ -0,0 +1,37 @@ +From 7f42013d33 Mon Sep 17 00:00:00 2001 +From: Nathan Scott +Subject: [PATCH] libpcp_web: add numinst overflow check in pmDiscoverDecodeMetaInDom (CWE-125/190) + +Defense-in-depth for the __pmLogLoadInDom streaming path fix (commit 1). +When __pmLogLoadInDom is called with acp=NULL from the pmproxy discover +code, a garbage numinst value read from a too-small buffer could be +passed to calloc(numinst, sizeof(char *)), causing an integer overflow +in the allocation size. + +Add explicit validation that numinst > 0 and does not overflow SIZE_MAX +before the calloc in pmDiscoverDecodeMetaInDom(). The primary fix +(rlen and numinst validation in __pmLogLoadInDom itself) prevents this +value from being garbage in the first place. + +Reported-by: Francisco Alisson Bezerra, TIM Security Red Team +Reported-by: Lucas Gabriel Alves, TIM Security Red Team +Reported-by: Massimiliano Brolli, TIM Security Red Team + +Co-Authored-By: Claude Opus 4.6 (1M context) +--- +diff --git a/src/libpcp_web/src/discover.c b/src/libpcp_web/src/discover.c +index 13781ded6f..6c5a2e9157 100644 +--- a/src/libpcp_web/src/discover.c ++++ b/src/libpcp_web/src/discover.c +@@ -2276,6 +2276,11 @@ pmDiscoverDecodeMetaInDom(__int32_t *buf, int len, int type, __pmTimestamp *tsp, + */ + char **namelist; + int i; ++ if (lid.numinst <= 0 || ++ (size_t)lid.numinst > SIZE_MAX / sizeof(char *)) { ++ __pmFreeLogInDom(&lid); ++ return -EINVAL; ++ } + namelist = (char **)calloc(lid.numinst, sizeof(char *)); + if (namelist == NULL) { + pmNoMem(__FUNCTION__, lid.numinst * sizeof(char *), PM_RECOV_ERR); diff --git a/pcp-7.1.5-OOB-pmLogLoadLabelSet.patch b/pcp-7.1.5-OOB-pmLogLoadLabelSet.patch new file mode 100644 index 0000000..1bc879a --- /dev/null +++ b/pcp-7.1.5-OOB-pmLogLoadLabelSet.patch @@ -0,0 +1,102 @@ +From ccd1bb1679 Mon Sep 17 00:00:00 2001 +From: Nathan Scott +Subject: [PATCH] libpcp: fix OOB read in __pmLogLoadLabelSet (CWE-125) + +__pmLogLoadLabelSet() reads timestamp, type, ident, and nsets fields +from tbuf at sequential offsets without checking that rlen is large +enough to contain them. When the pmproxy logger servlet delivers a +TYPE_LABEL record with hdr.len=13 (minimum accepted by the dispatcher), +rlen=1 and the function reads 20-24 bytes from a 1-byte buffer. + +Fix: add minimum-length guard at the top of __pmLogLoadLabelSet() +using LABELSET_V3_MINRLEN / LABELSET_V2_MINRLEN macros derived from +the on-disk __pmExtLabelSet_v3/v2 struct sizes (minus the len+type +header that rlen excludes). + +Test coverage will be added in a consolidated pducrash.c extension +covering vulns 9-13. + +Reported-by: Francisco Alisson Bezerra, TIM Security Red Team +Reported-by: Lucas Gabriel Alves, TIM Security Red Team +Reported-by: Massimiliano Brolli, TIM Security Red Team + +Co-Authored-By: Claude Opus 4.6 (1M context) +--- +diff --git a/src/libpcp/src/e_labels.c b/src/libpcp/src/e_labels.c +index 342cb9163d..0e18c72aed 100644 +--- a/src/libpcp/src/e_labels.c ++++ b/src/libpcp/src/e_labels.c +@@ -56,6 +56,10 @@ typedef struct { + /* will be expanded if nsets > 0 */ + } __pmExtLabelSet_v2; + ++/* Minimum rlen (record body without len+type header) to read fixed fields */ ++#define LABELSET_V3_MINRLEN (sizeof(__pmExtLabelSet_v3) - 2 * sizeof(__int32_t)) ++#define LABELSET_V2_MINRLEN (sizeof(__pmExtLabelSet_v2) - 2 * sizeof(__int32_t)) ++ + /* + * pack a set of labels into a physical metadata record + * - lcp required to provide archive version +@@ -210,6 +214,23 @@ __pmLogLoadLabelSet(char *tbuf, int rlen, int rtype, __pmTimestamp *stamp, + *nsetsp = 0; + *labelsetsp = NULL; + ++ if (rtype == TYPE_LABEL_V2) { ++ if (rlen < (int)LABELSET_V2_MINRLEN) { ++ if (pmDebugOptions.logmeta) ++ fprintf(stderr, "%s: v2 rlen=%d too small (min=%d)\n", ++ __FUNCTION__, rlen, (int)LABELSET_V2_MINRLEN); ++ return PM_ERR_LOGREC; ++ } ++ } ++ else { ++ if (rlen < (int)LABELSET_V3_MINRLEN) { ++ if (pmDebugOptions.logmeta) ++ fprintf(stderr, "%s: v3 rlen=%d too small (min=%d)\n", ++ __FUNCTION__, rlen, (int)LABELSET_V3_MINRLEN); ++ return PM_ERR_LOGREC; ++ } ++ } ++ + k = 0; + if (rtype == TYPE_LABEL_V2) { + __pmLoadTimeval((__int32_t *)&tbuf[k], stamp); +diff --git a/src/libpcp3/src/e_labels.c b/src/libpcp3/src/e_labels.c +index 8e94a112b0..169817266b 100644 +--- a/src/libpcp3/src/e_labels.c ++++ b/src/libpcp3/src/e_labels.c +@@ -56,6 +56,10 @@ typedef struct { + /* will be expanded if nsets > 0 */ + } __pmExtLabelSet_v2; + ++/* Minimum rlen (record body without len+type header) to read fixed fields */ ++#define LABELSET_V3_MINRLEN (sizeof(__pmExtLabelSet_v3) - 2 * sizeof(__int32_t)) ++#define LABELSET_V2_MINRLEN (sizeof(__pmExtLabelSet_v2) - 2 * sizeof(__int32_t)) ++ + /* + * pack a set of labels into a physical metadata record + * - lcp required to provide archive version +@@ -215,6 +219,23 @@ __pmLogLoadLabelSet(char *tbuf, int rlen, int rtype, __pmTimestamp *stamp, + *nsetsp = 0; + *labelsetsp = NULL; + ++ if (rtype == TYPE_LABEL_V2) { ++ if (rlen < (int)LABELSET_V2_MINRLEN) { ++ if (pmDebugOptions.logmeta) ++ fprintf(stderr, "%s: v2 rlen=%d too small (min=%d)\n", ++ __FUNCTION__, rlen, (int)LABELSET_V2_MINRLEN); ++ return PM_ERR_LOGREC; ++ } ++ } ++ else { ++ if (rlen < (int)LABELSET_V3_MINRLEN) { ++ if (pmDebugOptions.logmeta) ++ fprintf(stderr, "%s: v3 rlen=%d too small (min=%d)\n", ++ __FUNCTION__, rlen, (int)LABELSET_V3_MINRLEN); ++ return PM_ERR_LOGREC; ++ } ++ } ++ + k = 0; + if (rtype == TYPE_LABEL_V2) { + __pmLoadTimeval((__int32_t *)&tbuf[k], stamp); diff --git a/pcp-7.1.5-pducrash-oob-tests.patch b/pcp-7.1.5-pducrash-oob-tests.patch new file mode 100644 index 0000000..814c434 --- /dev/null +++ b/pcp-7.1.5-pducrash-oob-tests.patch @@ -0,0 +1,157 @@ +From 7465d7cbdb Mon Sep 17 00:00:00 2001 +From: Nathan Scott +Subject: [PATCH] qa: extend pducrash with tests for vulns 9, 12, 13 + +Add three new test functions to pducrash.c exercising the OOB read +fixes: + +- decode_log_labelset: calls __pmLogLoadLabelSet with rlen=1 for both + V2 and V3 record types, verifying the minimum-length guard rejects + undersized records (vuln 9) + +- decode_log_status_oob: crafts a PDU_LOG_STATUS V3 with hostname_len + extending past the PDU boundary, verifying the bounds check now runs + before strdup (vuln 12) + +- decode_instance_overshoot: crafts a PDU_INSTANCE claiming 2 entries + but only containing 1, where the alignment-padded advance pushes ip + past pdu_end, verifying the signed pointer guard catches it (vuln 13) + +Co-Authored-By: Claude Opus 4.6 (1M context) +--- +diff --git a/qa/513.out b/qa/513.out +index 7ccaa1f370..6ffe932b39 100644 +--- a/qa/513.out ++++ b/qa/513.out +@@ -277,6 +277,14 @@ QA output created by 513 + __pmLogLoadInDom: sts = -12373 (Corrupted record in a PCP archive) + [log_indom] checking out-of-range stridx with acp==NULL + __pmLogLoadInDom: sts = -12373 (Corrupted record in a PCP archive) ++[log_labelset] checking rlen too small for v3 header ++ __pmLogLoadLabelSet: sts = -12373 (Corrupted record in a PCP archive) ++[log_labelset] checking rlen too small for v2 header ++ __pmLogLoadLabelSet: sts = -12373 (Corrupted record in a PCP archive) ++[log_status_oob] checking hostname_len past PDU boundary ++ __pmDecodeLogStatus: sts = -12366 (IPC protocol failure) ++[instance_overshoot] checking alignment overshoot past pdu_end ++ __pmDecodeInstance: sts = -12366 (IPC protocol failure) + === filtered valgrind report === + Memcheck, a memory error detector + Command: src/pducrash +diff --git a/qa/src/pducrash.c b/qa/src/pducrash.c +index 76e1e0ce76..94a5f72d47 100644 +--- a/qa/src/pducrash.c ++++ b/qa/src/pducrash.c +@@ -1703,6 +1703,102 @@ decode_log_indom(const char *name) + } + } + ++/* ++ * Test __pmLogLoadLabelSet with undersized rlen (vuln 9). ++ */ ++static void ++decode_log_labelset(const char *name) ++{ ++ __pmTimestamp stamp; ++ pmLabelSet *sets = NULL; ++ int type, ident, nsets, sts; ++ char tiny[4]; ++ ++ fprintf(stderr, "[%s] checking rlen too small for v3 header\n", name); ++ memset(tiny, 0, sizeof(tiny)); ++ sts = __pmLogLoadLabelSet(tiny, 1, TYPE_LABEL, &stamp, &type, &ident, &nsets, &sets); ++ fprintf(stderr, " __pmLogLoadLabelSet: sts = %d (%s)\n", sts, pmErrStr(sts)); ++ if (sts >= 0 && sets) pmFreeLabelSets(sets, nsets); ++ ++ fprintf(stderr, "[%s] checking rlen too small for v2 header\n", name); ++ memset(tiny, 0, sizeof(tiny)); ++ sets = NULL; ++ sts = __pmLogLoadLabelSet(tiny, 1, TYPE_LABEL_V2, &stamp, &type, &ident, &nsets, &sets); ++ fprintf(stderr, " __pmLogLoadLabelSet: sts = %d (%s)\n", sts, pmErrStr(sts)); ++ if (sts >= 0 && sets) pmFreeLabelSets(sets, nsets); ++} ++ ++/* ++ * Test __pmDecodeLogStatus with hostname_len past PDU end (vuln 12). ++ */ ++static void ++decode_log_status_oob(const char *name) ++{ ++ __pmLoggerStatus *log; ++ int sts; ++ int save_ipc_version = __pmVersionIPC(0); ++ struct log_sts { ++ __pmPDUHdr hdr; ++ __int32_t buf[20+2*PM_LOG_MAXHOSTLEN+2*PM_TZ_MAXLEN]; ++ } *log_sts; ++ ++ __pmSetVersionIPC(0, LOG_PDU_VERSION3); ++ log_sts = (struct log_sts *)malloc(sizeof(*log_sts)); ++ ++ fprintf(stderr, "[%s] checking hostname_len past PDU boundary\n", name); ++ memset(log_sts, 0, sizeof(*log_sts)); ++ log_sts->hdr.len = 100; ++ log_sts->hdr.type = PDU_LOG_STATUS; ++ /* buf[13] is pmcd_hostname_len in V3 — set to extend past the PDU */ ++ log_sts->buf[13] = htonl(90); ++ sts = __pmDecodeLogStatus((__pmPDU *)log_sts, &log); ++ fprintf(stderr, " __pmDecodeLogStatus: sts = %d (%s)\n", sts, pmErrStr(sts)); ++ if (sts == 0) __pmFreeLogStatus(log, 1); ++ ++ free(log_sts); ++ __pmSetVersionIPC(0, save_ipc_version); ++} ++ ++/* ++ * Test __pmDecodeInstance with alignment-padding overshoot (vuln 13). ++ * Craft a PDU_INSTANCE with numinst=2 where the first entry's namelen ++ * causes the alignment-padded advance to push ip past pdu_end. ++ */ ++static void ++decode_instance_overshoot(const char *name) ++{ ++ pmInResult *inresult; ++ int sts; ++ struct { ++ __pmPDUHdr hdr; ++ pmInDom indom; ++ int numinst; ++ /* entry 0: inst + namelen + name (padded) */ ++ int inst0; ++ int namelen0; ++ char name0[4]; /* padded to 4 bytes */ ++ /* entry 1 would start here — but PDU ends before it */ ++ } *pdu; ++ ++ pdu = (typeof(pdu))malloc(sizeof(*pdu)); ++ ++ fprintf(stderr, "[%s] checking alignment overshoot past pdu_end\n", name); ++ memset(pdu, 0, sizeof(*pdu)); ++ pdu->hdr.len = sizeof(*pdu); ++ pdu->hdr.type = PDU_INSTANCE; ++ pdu->numinst = htonl(2); /* claim 2 entries but only room for 1 */ ++ pdu->inst0 = htonl(0); ++ pdu->namelen0 = htonl(3); /* 3 bytes + pad to 4 = alignment overshoot */ ++ pdu->name0[0] = 'a'; ++ pdu->name0[1] = 'b'; ++ pdu->name0[2] = 'c'; ++ sts = __pmDecodeInstance((__pmPDU *)pdu, &inresult); ++ fprintf(stderr, " __pmDecodeInstance: sts = %d (%s)\n", sts, pmErrStr(sts)); ++ if (sts >= 0) __pmFreeInResult(inresult); ++ ++ free(pdu); ++} ++ + typedef void (*decode_t)(const char *); + + struct pdu { +@@ -1738,6 +1834,9 @@ struct pdu { + { "desc_ids", decode_desc_ids }, + { "descs", decode_descs }, + { "log_indom", decode_log_indom }, ++ { "log_labelset", decode_log_labelset }, ++ { "log_status_oob", decode_log_status_oob }, ++ { "instance_overshoot", decode_instance_overshoot }, + }; + + int diff --git a/pcp-7.1.5-pmdaroot-peer-credentials.patch b/pcp-7.1.5-pmdaroot-peer-credentials.patch new file mode 100644 index 0000000..f176952 --- /dev/null +++ b/pcp-7.1.5-pmdaroot-peer-credentials.patch @@ -0,0 +1,112 @@ +From 2a4bd9f81a Mon Sep 17 00:00:00 2001 +From: Nathan Scott +Subject: [PATCH] pmdaroot: add peer credential verification on Unix socket (CWE-403) + +Defense-in-depth for the FD_CLOEXEC fix: verify the UID of connecting +clients on the pmdaroot Unix socket using SO_PEERCRED (Linux) or +getpeereid (macOS/FreeBSD). Only root (UID 0) and the PCP service +user (typically 'pcp') are permitted to connect. Connections from +other UIDs are rejected with a log message. + +This prevents exploitation even if the pmdaroot socket fd were to +leak to an unprivileged process through a path not covered by +FD_CLOEXEC (e.g., direct socket file access). + +The PCP service UID is resolved once at startup via pmGetUsername() +and getpwnam(), cached in a static for use in the accept path. + +Co-Authored-By: Claude Opus 4.6 (1M context) +--- +diff --git a/src/pmdas/root/root.c b/src/pmdas/root/root.c +index 7a900c9826..4e70454b70 100644 +--- a/src/pmdas/root/root.c ++++ b/src/pmdas/root/root.c +@@ -23,6 +23,9 @@ + #include "docker.h" + #include "podman.h" + #include "domain.h" ++#if defined(HAVE_PWD_H) ++#include ++#endif + + #ifndef S_IRWXU + /* +@@ -37,6 +40,7 @@ static char socket_path[MAXPATHLEN]; + static __pmSockAddr *socket_addr; + static int socket_fd = -1; + static int pmcd_fd = -1; ++static uid_t pcp_uid; + + static __pmFdSet connected_fds; + int root_maximum_fd; +@@ -461,6 +465,42 @@ root_accept_client(void) + exit(1); + } + } ++#if defined(HAVE_STRUCT_UCRED) ++ { ++ struct ucred cred; ++ __pmSockLen len = sizeof(cred); ++ ++ if (getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &cred, &len) == 0) { ++ if (cred.uid != 0 && cred.uid != pcp_uid) { ++ pmNotifyErr(LOG_ERR, ++ "root_accept_client: rejected uid=%d (expected root or pcp[%d])\n", ++ cred.uid, pcp_uid); ++ close(fd); ++ root_client[i].fd = -1; ++ root_delete_client(&root_client[i]); ++ return NULL; ++ } ++ } ++ } ++#elif defined(HAVE_GETPEEREID) ++ { ++ uid_t uid; ++ gid_t gid; ++ ++ if (getpeereid(fd, &uid, &gid) == 0) { ++ if (uid != 0 && uid != pcp_uid) { ++ pmNotifyErr(LOG_ERR, ++ "root_accept_client: rejected uid=%d (expected root or pcp[%d])\n", ++ uid, pcp_uid); ++ close(fd); ++ root_client[i].fd = -1; ++ root_delete_client(&root_client[i]); ++ return NULL; ++ } ++ } ++ } ++#endif ++ + if (fd > root_maximum_fd) + root_maximum_fd = fd; + __pmFD_SET(fd, &connected_fds); +@@ -796,6 +836,19 @@ root_main(pmdaInterface *dp) + } + } + ++static void ++root_get_pcp_uid(void) ++{ ++#if defined(HAVE_PWD_H) ++ char *username; ++ struct passwd *pw; ++ ++ pmGetUsername(&username); ++ if ((pw = getpwnam(username)) != NULL) ++ pcp_uid = pw->pw_uid; ++#endif ++} ++ + static void + root_check_user(void) + { +@@ -815,6 +868,7 @@ static void + root_prep(void) + { + root_check_user(); ++ root_get_pcp_uid(); + root_setup_socket(); + atexit(root_close_socket); + } diff --git a/pcp-7.1.5-pmieconf-command-injection.patch b/pcp-7.1.5-pmieconf-command-injection.patch new file mode 100644 index 0000000..6ace388 --- /dev/null +++ b/pcp-7.1.5-pmieconf-command-injection.patch @@ -0,0 +1,142 @@ +From cdc9676ab6 Mon Sep 17 00:00:00 2001 +From: Nathan Scott +Subject: [PATCH] pmieconf: fix command injection via $HOME and -f (CWE-78) + +The write_pmiefile() function constructed a shell command via +pmsprintf("/bin/mkdir -p %s", fname) and passed it to system(). +The fname value derives from either $HOME or the -f command-line +argument without sanitization, enabling command injection through +shell metacharacters in the path. + +Fix: replace system("/bin/mkdir -p ...") with __pmMakePath() which +creates directories recursively using mkdir() syscalls directly, +with no shell involvement. + +Add qa/2103 verifying that legitimate directory creation works and +that shell metacharacters in -f and $HOME paths do not result in +command execution. + +Reported-by: Francisco Alisson Bezerra, TIM Security Red Team +Reported-by: Lucas Gabriel Alves, TIM Security Red Team +Reported-by: Massimiliano Brolli, TIM Security Red Team + +Co-Authored-By: Claude Opus 4.6 (1M context) +--- +diff --git a/qa/2103 b/qa/2103 +new file mode 100755 +index 0000000000..c068ae92f0 +--- /dev/null ++++ b/qa/2103 +@@ -0,0 +1,62 @@ ++#!/bin/sh ++# PCP QA Test No. 2103 ++# Verify pmieconf does not execute shell metacharacters in -f path ++# (CWE-78 fix verification) ++# ++# Copyright (c) 2026 Red Hat. All Rights Reserved. ++# ++ ++seq=`basename $0` ++echo "QA output created by $seq" ++ ++# get standard environment, filters and checks ++. ./common.product ++. ./common.filter ++. ./common.check ++ ++which pmieconf >/dev/null 2>&1 || _notrun "pmieconf not installed" ++ ++_cleanup() ++{ ++ cd $here ++ $sudo rm -rf $tmp $tmp.* ++} ++ ++status=0 # success is the default! ++trap "_cleanup; exit \$status" 0 1 2 3 15 ++ ++# real QA test starts here ++ ++echo "=== normal -f path should work ===" ++mkdir -p $tmp.dir ++pmieconf -f $tmp.dir/subdir/test.pmie modify global delta "2min" >$tmp.out 2>&1 ++_sts=$? ++if [ -f $tmp.dir/subdir/test.pmie ]; then ++ echo "directory creation and file write succeeded" ++else ++ echo "FAIL: file not created (exit=$_sts)" ++ cat $tmp.out ++fi ++ ++echo ++echo "=== -f path with semicolon should not execute commands ===" ++_bad="$tmp.dir/bad;touch $tmp.dir/pwned" ++pmieconf -f "$_bad" modify global delta "2min" >$tmp.out 2>&1 ++if [ -f "$tmp.dir/pwned" ]; then ++ echo "FAIL: shell metacharacter was executed" ++else ++ echo "no command execution from semicolon in path" ++fi ++ ++echo ++echo "=== verify no injected file from HOME variable ===" ++_badhome="$tmp.dir/home;touch $tmp.dir/pwned2" ++HOME="$_badhome" pmieconf modify global delta "2min" >$tmp.out 2>&1 ++if [ -f "$tmp.dir/pwned2" ]; then ++ echo "FAIL: shell metacharacter in HOME was executed" ++else ++ echo "no command execution from HOME injection" ++fi ++ ++# success, all done ++exit +diff --git a/qa/2103.out b/qa/2103.out +new file mode 100644 +index 0000000000..a5054675e2 +--- /dev/null ++++ b/qa/2103.out +@@ -0,0 +1,9 @@ ++QA output created by 2103 ++=== normal -f path should work === ++directory creation and file write succeeded ++ ++=== -f path with semicolon should not execute commands === ++no command execution from semicolon in path ++ ++=== verify no injected file from HOME variable === ++no command execution from HOME injection +diff --git a/qa/group b/qa/group +index e83b623c96..16c5aa35e3 100644 +--- a/qa/group ++++ b/qa/group +@@ -2377,5 +2377,6 @@ suse + 2104 libpcp local security + 2101 pmda.sockets local security + 2102 pmlogmv local security ++2103 pmieconf local security + 4751 libpcp threads valgrind local pcp helgrind + 9000 other local +diff --git a/src/pmieconf/rules.c b/src/pmieconf/rules.c +index b571c5f4c0..34c701ae8b 100644 +--- a/src/pmieconf/rules.c ++++ b/src/pmieconf/rules.c +@@ -1808,7 +1808,6 @@ write_pmiefile(char *program, int autocreate) + { + time_t now = time(NULL); + char *p, *msg = NULL; +- char buf[MAXPATHLEN+10]; + char *fname = get_pmiefile(); + FILE *fp; + int i; +@@ -1819,9 +1818,8 @@ write_pmiefile(char *program, int autocreate) + + *p = '\0'; /* p is the dirname of fname */ + if (stat(fname, &sbuf) < 0) { +- pmsprintf(buf, sizeof(buf), "/bin/mkdir -p %s", fname); +- if (system(buf) < 0) { +- pmsprintf(errmsg, sizeof(errmsg), "failed to create directory \"%s\"", p); ++ if (__pmMakePath(fname, 0755) < 0) { ++ pmsprintf(errmsg, sizeof(errmsg), "failed to create directory \"%s\"", fname); + return errmsg; + } + } diff --git a/pcp-7.1.5-pmlogmv-command-injection.patch b/pcp-7.1.5-pmlogmv-command-injection.patch new file mode 100644 index 0000000..8cfa52c --- /dev/null +++ b/pcp-7.1.5-pmlogmv-command-injection.patch @@ -0,0 +1,549 @@ +From dc73ec0f57 Mon Sep 17 00:00:00 2001 +From: Nathan Scott +Subject: [PATCH] pmlogmv: fix command injection in pmlogcp/pmlogmv (CWE-78) + +The do_link() function used system("cp src dst") to copy archive files +when link() fails with EXDEV. The source filename was not validated by +check_name() and was embedded directly into the shell command, enabling +command injection via crafted archive filenames. The do_checksum() +function similarly used system() for command detection and popen() for +checksum execution. + +Fix: +- Replace system("cp ...") with copy_file() using open/read/write + syscalls directly, eliminating shell involvement entirely +- Replace system("if which ...") checksum detection with access() checks +- Replace popen("md5sum +--- +diff --git a/qa/2102 b/qa/2102 +new file mode 100755 +index 0000000000..5f8a4ef1cc +--- /dev/null ++++ b/qa/2102 +@@ -0,0 +1,67 @@ ++#!/bin/sh ++# PCP QA Test No. 2102 ++# Verify pmlogcp/pmlogmv reject shell metacharacters in filenames ++# and that normal copy/move operations still work (CWE-78 fix) ++# ++# Copyright (c) 2026 Red Hat. All Rights Reserved. ++# ++ ++seq=`basename $0` ++echo "QA output created by $seq" ++ ++# get standard environment, filters and checks ++. ./common.product ++. ./common.filter ++. ./common.check ++ ++_cleanup() ++{ ++ 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 "s,$tmp,TMP,g" \ ++ -e "s/pmlogcp/TOOL/" \ ++ -e "s/pmlogmv/TOOL/" \ ++ # end ++} ++ ++# real QA test starts here ++ ++echo "=== normal pmlogcp should succeed ===" ++pmlogcp tmparch/foo $tmp.copy1 2>&1 | _filter ++if [ -f $tmp.copy1.meta ] || [ -f $tmp.copy1.0 ]; then ++ echo "copy succeeded" ++else ++ echo "FAIL: copy did not create files" ++fi ++ ++echo ++echo "=== pmlogcp with backtick in destination should be rejected ===" ++pmlogcp tmparch/foo "$tmp.bad\`id\`" >$tmp.out 2>&1 ++_sts=$? ++_filter <$tmp.out ++echo "exit status: $_sts" ++ ++echo ++echo "=== pmlogcp with semicolon in destination should be rejected ===" ++pmlogcp tmparch/foo "$tmp.bad;id" >$tmp.out 2>&1 ++_sts=$? ++_filter <$tmp.out ++echo "exit status: $_sts" ++ ++echo ++echo "=== pmlogcp with dollar in destination should be rejected ===" ++pmlogcp tmparch/foo '$tmp.bad${IFS}' >$tmp.out 2>&1 ++_sts=$? ++_filter <$tmp.out ++echo "exit status: $_sts" ++ ++# success, all done ++exit +diff --git a/qa/2102.out b/qa/2102.out +new file mode 100644 +index 0000000000..5a3fec6ca8 +--- /dev/null ++++ b/qa/2102.out +@@ -0,0 +1,15 @@ ++QA output created by 2102 ++=== normal pmlogcp should succeed === ++copy succeeded ++ ++=== pmlogcp with backtick in destination should be rejected === ++TOOL: name (TMP.bad`id`) unsafe [shell metacharacter '`'] ++exit status: 1 ++ ++=== pmlogcp with semicolon in destination should be rejected === ++TOOL: name (TMP.bad;id) unsafe [shell metacharacter ';'] ++exit status: 1 ++ ++=== pmlogcp with dollar in destination should be rejected === ++TOOL: name ($tmp.bad${IFS}) unsafe [shell metacharacter '$'] ++exit status: 1 +diff --git a/qa/group b/qa/group +index 2c6529b718..e83b623c96 100644 +--- a/qa/group ++++ b/qa/group +@@ -2376,5 +2376,6 @@ suse + 2100 pmproxy local security + 2104 libpcp local security + 2101 pmda.sockets local security ++2102 pmlogmv local security + 4751 libpcp threads valgrind local pcp helgrind + 9000 other local +diff --git a/src/pmlogmv/pmlogmv.c b/src/pmlogmv/pmlogmv.c +index 8f1ce36e5a..48a89831c1 100644 +--- a/src/pmlogmv/pmlogmv.c ++++ b/src/pmlogmv/pmlogmv.c +@@ -13,8 +13,9 @@ + */ + + /* +- * pmlogmv - move/rename PCP archives +- * pmlogcp - copy PCP archives ++ * pmlogmv - move/rename a PCP archive ++ * pmlogcp - copy a PCP archive ++ * pmlogls - list files in a PCP archive + */ + + #include +@@ -28,7 +29,10 @@ + + static int myoverrides(int, pmOptions *); + +-static pmLongOptions longopts[] = { ++/* ++ * options for pmlogmv|pmlogcp ++ */ ++static pmLongOptions longopts_mvcp[] = { + PMAPI_OPTIONS_HEADER("Options"), + PMOPT_DEBUG, + { "checksum", 0, 'c', 0, "checksum all source and destintion files when copying" }, +@@ -39,18 +43,39 @@ static pmLongOptions longopts[] = { + PMAPI_OPTIONS_END + }; + +-static pmOptions opts = { ++static pmOptions opts_mvcp = { + .short_options = "cD:fNV?", +- .long_options = longopts, ++ .long_options = longopts_mvcp, + .short_usage = "[options] srcname dstname", + .override = myoverrides + }; + ++/* ++ * options for pmlogls ++ */ ++static pmLongOptions longopts_ls[] = { ++ PMAPI_OPTIONS_HEADER("Options"), ++ PMOPT_DEBUG, ++ { "verbose", 0, 'V', 0, "increase diagnostic verbosity" }, ++ PMOPT_HELP, ++ PMAPI_OPTIONS_END ++}; ++ ++static pmOptions opts_ls = { ++ .short_options = "D:V?", ++ .long_options = longopts_ls, ++ .short_usage = "[options] srcname", ++ .override = myoverrides ++}; ++ ++static pmOptions *opts; ++ + static char *progname; + ++static int mode; /* MV, CP or LS depending on argv[0] */ + #define MV 1 + #define CP 2 +-static int mode; /* MV or CP depending on argv[0] */ ++#define LS 3 + + static int showme = 0; + static int verbose = 0; +@@ -78,15 +103,22 @@ myoverrides(int opt, pmOptions *optsp) + return 0; + } + ++/* ++ * Defense-in-depth: reject filenames containing shell metacharacters. ++ * The copy and checksum paths no longer use system()/popen() so these ++ * characters are not directly dangerous, but archive names containing ++ * them are almost certainly bogus and this guards against future code ++ * paths that might reintroduce shell interpretation. ++ */ + static int + check_name(char *name) + { +- char *meta = " $?*[(|;&<>"; ++ char *meta = " $?*[(|;&<>`{}\\!\n\t"; + char *p; + + for (p = meta; *p; p++) { + if (strchr(name, *p) != NULL) { +- fprintf(stderr, "%s: dstname (%s) unsafe [shell metacharacter '%c']\n", progname, name, *p); ++ fprintf(stderr, "%s: name (%s) unsafe [shell metacharacter '%c']\n", progname, name, *p); + return -1; + } + } +@@ -152,7 +184,6 @@ setup_sufftab(void) + void + do_checksum(const char *file, char *sum) + { +- char cmd[2*MAXPATHLEN+20]; + static char *executable = NULL; + FILE *fp; + static int trunc_warn = 0; +@@ -163,43 +194,49 @@ do_checksum(const char *file, char *sum) + * prefer md5sum, then sha256sum, then sha1sum, then sum, + * else do nothing + */ +- snprintf(cmd, sizeof(cmd), "if which md5sum >/dev/null 2>&1; then exit 0; fi; exit 1"); +- if (system(cmd) == 0) +- executable = "md5sum"; +- else { +- snprintf(cmd, sizeof(cmd), "if which sha256sum >/dev/null 2>&1; then exit 0; fi; exit 1"); +- if (system(cmd) == 0) +- executable = "sha256sum"; +- else { +- snprintf(cmd, sizeof(cmd), "if which sha1sum >/dev/null 2>&1; then exit 0; fi; exit 1"); +- if (system(cmd) == 0) +- executable = "sha1sum"; +- else { +- snprintf(cmd, sizeof(cmd), "if which sum >/dev/null 2>&1; then exit 0; fi; exit 1"); +- if (system(cmd) == 0) +- executable = "sum"; +- else { +- executable = "none"; +- fprintf(stderr, "%s: warning: no checksum command found, checksums skipped\n", progname); +- } +- } ++ static const char *candidates[] = { ++ "md5sum", "sha256sum", "sha1sum", "sum", NULL ++ }; ++ const char **cp; ++ char path[MAXPATHLEN]; ++ ++ executable = "none"; ++ for (cp = candidates; *cp != NULL; cp++) { ++ snprintf(path, sizeof(path), "/usr/bin/%s", *cp); ++ if (access(path, X_OK) == 0) { ++ executable = (char *)*cp; ++ break; ++ } ++ snprintf(path, sizeof(path), "/usr/sbin/%s", *cp); ++ if (access(path, X_OK) == 0) { ++ executable = (char *)*cp; ++ break; + } + } ++ if (strcmp(executable, "none") == 0) ++ fprintf(stderr, "%s: warning: no checksum command found, checksums skipped\n", progname); + if (verbose && strcmp(executable, "none") != 0) + printf("checksum cmd: %s\n", executable); + } + sum[0] = '\0'; + if (strcmp(executable, "none") == 0) + return; +- snprintf(cmd, sizeof(cmd), "%s <%s", executable, file); +- if ((fp = popen(cmd, "r")) == NULL) { +- /* +- * abandon checksuming ... +- */ +- fprintf(stderr, "%s: pipe(\"%s\") failed: %s\n", progname, cmd, strerror(errno)); +- executable = "none"; ++ { ++ __pmExecCtl_t *argp = NULL; ++ int sts; ++ ++ if ((sts = __pmProcessAddArg(&argp, executable)) < 0 || ++ (sts = __pmProcessAddArg(&argp, file)) < 0) { ++ executable = "none"; ++ return; ++ } ++ if ((sts = __pmProcessPipe(&argp, "r", PM_EXEC_TOSS_NONE, &fp)) < 0) { ++ fprintf(stderr, "%s: __pmProcessPipe(\"%s\") failed: %s\n", progname, executable, pmErrStr(sts)); ++ executable = "none"; ++ return; ++ } + } +- else { ++ { + char *p = sum; + int c; + while ((c = fgetc(fp)) != EOF) { +@@ -208,9 +245,6 @@ do_checksum(const char *file, char *sum) + break; + } + if (p >= &sum[MAX_CHECKSUM]) { +- /* +- * avoid buffer overrun, report only once unless -V +- */ + if (trunc_warn++ == 0 || verbose) + fprintf(stderr, "%s: warning: checksum truncated after %d characters\n", progname, MAX_CHECKSUM); + *p = '\0'; +@@ -218,12 +252,55 @@ do_checksum(const char *file, char *sum) + } + *p++ = c; + } +- pclose(fp); ++ __pmProcessPipeClose(fp); ++ } ++} ++ ++/* ++ * copy a file using read/write - no shell involvement ++ */ ++static int ++copy_file(const char *src, const char *dst) ++{ ++ int sfd, dfd; ++ struct stat sbuf; ++ ssize_t nread, nwritten; ++ char buf[BUFSIZ]; ++ ++ if ((sfd = open(src, O_RDONLY)) < 0) ++ return -1; ++ if (fstat(sfd, &sbuf) < 0) { ++ close(sfd); ++ return -1; ++ } ++ if ((dfd = open(dst, O_WRONLY|O_CREAT|O_EXCL, sbuf.st_mode & 0777)) < 0) { ++ close(sfd); ++ return -1; ++ } ++ while ((nread = read(sfd, buf, sizeof(buf))) > 0) { ++ char *p = buf; ++ while (nread > 0) { ++ nwritten = write(dfd, p, nread); ++ if (nwritten < 0) { ++ close(sfd); ++ close(dfd); ++ unlink(dst); ++ return -1; ++ } ++ nread -= nwritten; ++ p += nwritten; ++ } ++ } ++ close(sfd); ++ if (nread < 0 || close(dfd) < 0) { ++ unlink(dst); ++ return -1; + } ++ return 0; + } + + /* +- * make link or copy for one physical file ++ * make link or make copy or list for one physical file + * return codes: + * 1: ok + * 0: source file not found +@@ -250,6 +327,10 @@ do_link(int vol) + } + if (access(src, F_OK) == 0) { + /* src exists ... off to the races */ ++ if (mode == LS) { ++ printf("%s\n", src); ++ return 1; ++ } + switch (vol) { + case PM_LOG_VOL_TI: + snprintf(dst, sizeof(src), "%s.index%s", dstname, *suff); +@@ -279,7 +360,6 @@ do_link(int vol) + #endif + /* pmlogcp or link() failed cross-device, need to copy ... */ + int sts; +- char cmd[2*MAXPATHLEN+60]; + char sum_src[MAX_CHECKSUM+1]; + char sum_dst[MAX_CHECKSUM+1]; + if (checksum) { +@@ -292,8 +372,7 @@ do_link(int vol) + printf("source checksum: %s\n", sum_src); + } + +- snprintf(cmd, sizeof(cmd), "cp %s %s", src, dst); +- if ((sts = system(cmd)) != 0) { ++ if ((sts = copy_file(src, dst)) != 0) { + fprintf(stderr, "%s: copy %s -> %s failed: %s\n", progname, src, dst, strerror(errno)); + return -1; + } +@@ -389,6 +468,9 @@ cleanup(int sig) + { + int i; + ++ if (mode == LS) ++ exit(0); ++ + if (sig != 0) { + fprintf(stderr, "Caught signal %d\n", sig); + verbose = 1; +@@ -440,19 +522,27 @@ main(int argc, char **argv) + pmSetProgname(argv[0]); + progname = pmGetProgname(); + +- if (strcmp(progname, "pmlogmv") == 0) ++ if (strcmp(progname, "pmlogmv") == 0) { + mode = MV; +- else if (strcmp(progname, "pmlogcp") == 0) ++ opts = &opts_mvcp; ++ } ++ else if (strcmp(progname, "pmlogcp") == 0) { + mode = CP; ++ opts = &opts_mvcp; ++ } ++ else if (strcmp(progname, "pmlogls") == 0) { ++ mode = LS; ++ opts = &opts_ls; ++ } + else { +- fprintf(stderr, "%s: Arrgh, not pmlogmv nor pmlogcp so I don't know who I am!\n", progname); ++ fprintf(stderr, "%s: Arrgh, not pmlogmv nor pmlogcp nor pmlogls so I don't know who I am!\n", progname); + return(1); + } + + setlinebuf(stdout); + setlinebuf(stderr); + +- while ((c = pmGetOptions(argc, argv, &opts)) != EOF) { ++ while ((c = pmGetOptions(argc, argv, opts)) != EOF) { + switch (c) { + + case 'c': /* checksum if copying */ +@@ -473,24 +563,27 @@ main(int argc, char **argv) + + case '?': + default: +- opts.errors++; ++ opts->errors++; + break; + } + } + +- if (opts.errors || opts.optind != argc-2) { +- pmUsageMessage(&opts); ++ if (opts->errors || ++ (mode != LS && opts->optind != argc-2) || ++ (mode == LS && opts->optind != argc-1)) { ++ pmUsageMessage(opts); + exit(1); + } + +- srcname = strdup(argv[opts.optind]); ++ srcname = strdup(argv[opts->optind]); + if (srcname == NULL) { + fprintf(stderr, "%s: malloc(srcname) failed!\n", progname); + exit(1); + } + + if ((sts = pmNewContext(PM_CONTEXT_ARCHIVE, srcname)) < 0) { +- fprintf(stderr, "%s: Cannot open archive \"%s\": %s\n", progname, srcname, pmErrStr(sts)); ++ if (mode != LS || verbose) ++ fprintf(stderr, "%s: Cannot open archive \"%s\": %s\n", progname, srcname, pmErrStr(sts)); + exit(1); + } + if ((ctxp = __pmHandleToPtr(sts)) == NULL) { +@@ -499,25 +592,35 @@ main(int argc, char **argv) + } + srcname = ctxp->c_archctl->ac_log->name; + +- opts.optind++; +- /* +- * default is that dstname is really the basename for the +- * destination archive +- */ +- snprintf(dstname, sizeof(dstname), "%s", argv[opts.optind]); +- sb.st_mode = 0; +- if (stat(argv[opts.optind], &sb) == 0 && S_ISDIR(sb.st_mode)) { +- /* +- * dstname is an existing directory ... append +- * basename of srcname ++ /* strip a leading "./" from the libpcp name */ ++ if (strncmp(srcname, "./", 2) == 0) ++ srcname += 2; ++ ++ if (mode != LS) { ++ opts->optind++; ++ /* ++ * default is that dstname is really the basename for the ++ * destination archive + */ +- snprintf(dstname, sizeof(dstname), "%s%c%s", +- argv[opts.optind], pmPathSeparator(), basename(srcname)); +- } ++ snprintf(dstname, sizeof(dstname), "%s", argv[opts->optind]); ++ sb.st_mode = 0; ++ if (stat(argv[opts->optind], &sb) == 0 && S_ISDIR(sb.st_mode)) { ++ /* ++ * dstname is an existing directory ... append ++ * basename of srcname ++ */ ++ snprintf(dstname, sizeof(dstname), "%s%c%s", ++ argv[opts->optind], pmPathSeparator(), basename(srcname)); ++ } + +- if (!force && check_name(dstname) < 0) { +- /* error reported in check_name() */ +- exit(1); ++ if (!force && check_name(dstname) < 0) { ++ /* error reported in check_name() */ ++ exit(1); ++ } ++ if (!force && check_name(srcname) < 0) { ++ /* error reported in check_name() */ ++ exit(1); ++ } + } + + if (setup_sufftab() < 0) { +@@ -556,6 +659,7 @@ main(int argc, char **argv) + do_unlink(0, srcname, PM_LOG_VOL_TI); + do_unlink(0, srcname, PM_LOG_VOL_META); + } ++ + return 0; + + /* fatal error once we're started ... remove any dstname files */ diff --git a/pcp-7.1.5-pmproxy-logger-auth.patch b/pcp-7.1.5-pmproxy-logger-auth.patch new file mode 100644 index 0000000..5491de7 --- /dev/null +++ b/pcp-7.1.5-pmproxy-logger-auth.patch @@ -0,0 +1,206 @@ +From 4121ae06f5 Mon Sep 17 00:00:00 2001 +From: Nathan Scott +Subject: [PATCH] pmproxy: add optional authentication for logger servlet + +The pmproxy logger servlet endpoints (/logger/label, /logger/meta, +/logger/index, /logger/volume) are registered unconditionally with no +authentication check, allowing any HTTP client to submit archive data. + +Add a new pmproxy.conf option [pmlogger] authenticate = true that +enables HTTP Basic authentication for all logger servlet requests. +When set, requests without valid credentials are rejected with +HTTP 403 Forbidden. Disabled by default to preserve existing behavior. + +This complements the global -S flag: -S requires authentication for +all servlets, while [pmlogger] authenticate = true targets only the +logger servlet endpoints. + +Co-Authored-By: Claude Opus 4.6 (1M context) +--- +diff --git a/qa/2108 b/qa/2108 +new file mode 100755 +index 0000000000..5e0d75c494 +--- /dev/null ++++ b/qa/2108 +@@ -0,0 +1,97 @@ ++#!/bin/sh ++# PCP QA Test No. 2108 ++# Verify pmproxy logger servlet authentication via pmproxy.conf ++# [pmlogger] authenticate = true ++# ++# Copyright (c) 2026 Red Hat. All Rights Reserved. ++# ++ ++seq=`basename $0` ++echo "QA output created by $seq" ++ ++# get standard environment, filters and checks ++. ./common.product ++. ./common.filter ++. ./common.check ++ ++which curl >/dev/null 2>&1 || _notrun "no curl executable installed" ++ ++_cleanup() ++{ ++ [ -n "$__pid" ] && kill $__pid 2>/dev/null ++ wait $__pid 2>/dev/null ++ cd $here ++ $sudo rm -rf $tmp $tmp.* ++} ++ ++status=0 # success is the default! ++__pid="" ++trap "_cleanup; exit \$status" 0 1 2 3 15 ++ ++# real QA test starts here ++ ++echo "=== logger servlet with authenticate = true ===" ++__port=`_find_free_port` ++cat >$tmp.conf </dev/null; then ++ echo "FAIL: pmproxy did not start" ++ exit ++fi ++ ++# unauthenticated POST to logger/label should be rejected ++__code=$(curl -s -o /dev/null -w '%{http_code}' \ ++ -X POST "http://localhost:$__port/logger/label" \ ++ -H 'Content-Type: application/octet-stream' \ ++ --data-binary 'dummy' 2>/dev/null) ++echo "unauthenticated POST /logger/label: HTTP $__code" ++ ++# unauthenticated GET to pmapi should still work (not logger servlet) ++__code=$(curl -s -o /dev/null -w '%{http_code}' \ ++ "http://localhost:$__port/pmapi/context?hostspec=localhost" 2>/dev/null) ++echo "unauthenticated GET /pmapi/context: HTTP $__code" ++ ++kill $__pid ++wait $__pid 2>/dev/null ++__pid="" ++ ++echo ++echo "=== logger servlet without authenticate (default) ===" ++__port=`_find_free_port` ++cat >$tmp.conf2 </dev/null; then ++ echo "FAIL: pmproxy did not start" ++ exit ++fi ++ ++# unauthenticated POST to logger/label should be allowed (will fail on bad data, not auth) ++__code=$(curl -s -o /dev/null -w '%{http_code}' \ ++ -X POST "http://localhost:$__port/logger/label" \ ++ -H 'Content-Type: application/octet-stream' \ ++ --data-binary 'dummy' 2>/dev/null) ++echo "unauthenticated POST /logger/label: HTTP $__code" ++ ++kill $__pid ++wait $__pid 2>/dev/null ++__pid="" ++ ++# success, all done ++exit +diff --git a/qa/2108.out b/qa/2108.out +new file mode 100644 +index 0000000000..8321285476 +--- /dev/null ++++ b/qa/2108.out +@@ -0,0 +1,7 @@ ++QA output created by 2108 ++=== logger servlet with authenticate = true === ++unauthenticated POST /logger/label: HTTP 403 ++unauthenticated GET /pmapi/context: HTTP 200 ++ ++=== logger servlet without authenticate (default) === ++unauthenticated POST /logger/label: HTTP 400 +diff --git a/qa/group b/qa/group +index 1103b1f526..bd2a2453d6 100644 +--- a/qa/group ++++ b/qa/group +@@ -2379,5 +2379,6 @@ suse + 2102 pmlogmv local security + 2103 pmieconf local security + 2107 libpcp local security ++2108 pmproxy local security + 4751 libpcp threads valgrind local pcp helgrind + 9000 other local +diff --git a/src/pmproxy/pmproxy.conf b/src/pmproxy/pmproxy.conf +index 52572ae647..36baa12f52 100644 +--- a/src/pmproxy/pmproxy.conf ++++ b/src/pmproxy/pmproxy.conf +@@ -122,6 +122,9 @@ stream.maxlen = 8640 + # allow REST API webhook receiving remote pmlogger(1) archive content + enabled = true + ++# require HTTP Basic authentication for logger servlet endpoints ++#authenticate = true ++ + # bypass persistent storage of pmlogger archives, use key server only + #cached = true + +diff --git a/src/pmproxy/src/logger.c b/src/pmproxy/src/logger.c +index 97c5082273..c5f383d247 100644 +--- a/src/pmproxy/src/logger.c ++++ b/src/pmproxy/src/logger.c +@@ -106,6 +106,9 @@ on_pmlogger_done(int status, void *arg) + if (status >= 0) { + code = HTTP_STATUS_OK; + body = pmlogger_success; ++ } else if (client->u.http.parser.status_code) { ++ code = client->u.http.parser.status_code; ++ body = pmlogger_failure; + } else { + if (status == -EEXIST) + code = HTTP_STATUS_CONFLICT; +@@ -139,6 +142,8 @@ on_pmlogger_info(pmLogLevel level, sds message, void *arg) + proxylog(level, message, baton->client->proxy); + } + ++static int pmlogger_authenticate; ++ + static pmLogGroupSettings pmlogger_settings = { + .callbacks.on_archive = on_pmlogger_archive, + .callbacks.on_done = on_pmlogger_done, +@@ -273,6 +278,10 @@ pmlogger_request_headers(struct client *client, struct dict *headers) + { + if (pmDebugOptions.http) + fprintf(stderr, "logger servlet headers (client=" PRINTF_P_PFX "%p)\n", client); ++ if (pmlogger_authenticate && ++ (!client->u.http.username || !client->u.http.password)) { ++ client->u.http.parser.status_code = HTTP_STATUS_FORBIDDEN; ++ } + return 0; + } + +@@ -378,6 +387,11 @@ pmlogger_servlet_setup(struct proxy *proxy) + { + mmv_registry_t *registry = proxymetrics(proxy, METRICS_LOGGROUP); + mmv_registry_t *logpaths = proxymetrics(proxy, METRICS_LOGPATHS); ++ sds value; ++ ++ if ((value = pmIniFileLookup(proxy->config, "pmlogger", "authenticate")) ++ && strcmp(value, "true") == 0) ++ pmlogger_authenticate = 1; + + PARAM_CLIENT = sdsnew("client"); + diff --git a/pcp-7.1.5-pmproxy-logger-meta-network.patch b/pcp-7.1.5-pmproxy-logger-meta-network.patch new file mode 100644 index 0000000..cf752e3 --- /dev/null +++ b/pcp-7.1.5-pmproxy-logger-meta-network.patch @@ -0,0 +1,183 @@ +From bf898b50ef Mon Sep 17 00:00:00 2001 +From: Nathan Scott +Subject: [PATCH] qa: add network-layer regression tests for pmproxy /logger/meta + +Adapted from PoC exploits provided by TIM Security Red Team to test +the pmproxy HTTP streaming path that pducrash.c does not exercise. + +qa/2109 starts pmproxy, creates a valid archive via POST /logger/label, +then POSTs three malformed metadata records to /logger/meta: +- Vuln 1: TYPE_INDOM with stridx=0x7FFFFFFF (OOB pointer deref) +- Vuln 9: TYPE_LABEL with hdr.len=13 (rlen=1, undersized for header) +- Vuln 10: TYPE_INDOM_DELTA with hdr.len=13 (OOB numinst read) + +Verifies pmproxy handles each gracefully and remains responsive. + +Co-Authored-By: Claude Opus 4.6 (1M context) +--- +diff --git a/qa/2109 b/qa/2109 +new file mode 100755 +index 0000000000..6423286d97 +--- /dev/null ++++ b/qa/2109 +@@ -0,0 +1,145 @@ ++#!/bin/sh ++# PCP QA Test No. 2109 ++# Verify pmproxy /logger/meta endpoint rejects malformed metadata ++# records without crashing (network-layer regression for vulns 1, 9, 10) ++# ++# Adapted from PoC exploits provided by TIM Security Red Team. ++# ++# Copyright (c) 2026 Red Hat. All Rights Reserved. ++# ++ ++seq=`basename $0` ++echo "QA output created by $seq" ++ ++# get standard environment, filters and checks ++. ./common.product ++. ./common.filter ++. ./common.check ++ ++which curl >/dev/null 2>&1 || _notrun "no curl executable installed" ++which python3 >/dev/null 2>&1 || _notrun "no python3 executable installed" ++ ++_cleanup() ++{ ++ [ -n "$__pid" ] && kill $__pid 2>/dev/null ++ wait $__pid 2>/dev/null ++ cd $here ++ $sudo rm -rf $tmp $tmp.* ++} ++ ++status=0 # success is the default! ++__pid="" ++trap "_cleanup; exit \$status" 0 1 2 3 15 ++ ++__port=`_find_free_port` ++mkdir -p $tmp.archdir ++cat >$tmp.conf </dev/null; then ++ echo "FAIL: pmproxy did not start" ++ exit ++fi ++ ++_filter_archive_id() ++{ ++ sed -e 's/archive created: [0-9]*/archive created: ARCHIVE_ID/' ++} ++ ++# real QA test starts here ++python3 - $__port <<'PYEOF' | _filter_archive_id ++import http.client, json, struct, sys, time ++ ++PORT = int(sys.argv[1]) ++ ++def post(path, body): ++ c = http.client.HTTPConnection("localhost", PORT, timeout=10) ++ c.request("POST", path, body=body, ++ headers={"Content-Type": "application/octet-stream"}) ++ try: ++ r = c.getresponse() ++ data = r.read() ++ c.close() ++ return r.status, data ++ except http.client.RemoteDisconnected: ++ return 0, b"connection lost" ++ ++# create a valid archive via POST /logger/label ++PM_LOG_MAGIC = 0x50052600 ++LABEL_MAGIC = PM_LOG_MAGIC | 0x02 ++LABEL_V2_SIZE = 4 + 4 + 8 + 4 + 64 + 40 ++TOTAL_SIZE = LABEL_V2_SIZE + 8 ++ ++hostname = b'testhost\x00' + b'\x00' * (64 - 9) ++timezone = b'UTC\x00' + b'\x00' * (40 - 4) ++ ++label = struct.pack(">I", LABEL_MAGIC) ++label += struct.pack(">i", 1337) ++label += struct.pack(">ii", int(time.time()), 0) ++label += struct.pack(">i", 0) ++label += hostname + timezone ++ ++label_body = struct.pack(">i", TOTAL_SIZE) + label + struct.pack(">i", TOTAL_SIZE) ++ ++status, resp = post("/logger/label", label_body) ++if status != 200: ++ print(f"FAIL: /logger/label returned HTTP {status}") ++ sys.exit(0) ++archive_id = json.loads(resp)["archive"] ++print(f"archive created: {archive_id}") ++ ++# --- Vuln 1: TYPE_INDOM with OOB stridx --- ++print("=== vuln 1: TYPE_INDOM with OOB stridx ===") ++TYPE_INDOM = 5 ++body = ( ++ struct.pack(">II", 0, 0) + # sec[2] ++ struct.pack(">I", 0) + # nsec ++ struct.pack(">I", 1) + # indom ++ struct.pack(">I", 1) + # numinst=1 ++ struct.pack(">I", 0) + # instlist[0] ++ struct.pack(">I", 0x7FFFFFFF) + # stridx[0] OOB ++ b'\x00' ++) ++total = 8 + len(body) + 4 ++record = struct.pack(">II", total, TYPE_INDOM) + body + struct.pack(">I", total) ++status, resp = post(f"/logger/meta/{archive_id}", record) ++print(f"pmproxy responded (not crashed)") ++ ++# --- Vuln 9: TYPE_LABEL with rlen=1 (undersized) --- ++print("=== vuln 9: TYPE_LABEL with rlen=1 ===") ++TYPE_LABEL = 7 ++hdr_len = 13 ++record = struct.pack(">ii", hdr_len, TYPE_LABEL) + b'\x00' + struct.pack(">i", hdr_len) ++status, resp = post(f"/logger/meta/{archive_id}", record) ++print(f"pmproxy responded (not crashed)") ++ ++# --- Vuln 10: TYPE_INDOM_DELTA with rlen=1 (OOB numinst) --- ++print("=== vuln 10: TYPE_INDOM_DELTA with rlen=1 ===") ++TYPE_INDOM_DELTA = 6 ++hdr_len = 13 ++record = struct.pack(">ii", hdr_len, TYPE_INDOM_DELTA) + b'\x00' + struct.pack(">i", hdr_len) ++status, resp = post(f"/logger/meta/{archive_id}", record) ++print(f"pmproxy responded (not crashed)") ++ ++# verify pmproxy is still alive after all malformed records ++try: ++ c = http.client.HTTPConnection("localhost", PORT, timeout=5) ++ c.request("GET", "/pmapi/ping") ++ r = c.getresponse() ++ r.read() ++ c.close() ++ print("=== pmproxy still responding after all tests ===") ++except Exception: ++ print("FAIL: pmproxy is not responding") ++PYEOF ++ ++# success, all done ++exit +diff --git a/qa/2109.out b/qa/2109.out +new file mode 100644 +index 0000000000..9918d5f025 +--- /dev/null ++++ b/qa/2109.out +@@ -0,0 +1,9 @@ ++QA output created by 2109 ++archive created: ARCHIVE_ID ++=== vuln 1: TYPE_INDOM with OOB stridx === ++pmproxy responded (not crashed) ++=== vuln 9: TYPE_LABEL with rlen=1 === ++pmproxy responded (not crashed) ++=== vuln 10: TYPE_INDOM_DELTA with rlen=1 === ++pmproxy responded (not crashed) ++=== pmproxy still responding after all tests === diff --git a/pcp-7.1.5-pmproxy-rest-certreqd.patch b/pcp-7.1.5-pmproxy-rest-certreqd.patch new file mode 100644 index 0000000..1f572cd --- /dev/null +++ b/pcp-7.1.5-pmproxy-rest-certreqd.patch @@ -0,0 +1,42 @@ +From 81a9efe96d Mon Sep 17 00:00:00 2001 +From: Nathan Scott +Subject: [PATCH] pmproxy: enforce -Q (CERT_REQD) for REST API connections + +The -Q flag (PM_SERVER_FEATURE_CERT_REQD) was only enforced in the +legacy PCP wire protocol path (deprecated.c). The modern HTTP/REST +API path had no check, allowing unauthenticated plain-HTTP clients +to access all endpoints even when -Q was specified. + +Add enforcement in on_headers_complete() alongside the existing -S +(CREDS_REQD) check: when CERT_REQD is active, reject requests where +the connection is not TLS or no client certificate was presented. +Returns HTTP 403 Forbidden. If OpenSSL is not compiled in, all +connections are rejected when -Q is set since TLS is unavailable. + +Co-Authored-By: Claude Opus 4.6 (1M context) +--- +diff --git a/src/pmproxy/src/http.c b/src/pmproxy/src/http.c +index 8da21a41d5..931bce59c1 100644 +--- a/src/pmproxy/src/http.c ++++ b/src/pmproxy/src/http.c +@@ -1129,6 +1129,20 @@ on_headers_complete(http_parser *request) + } + } + ++ /* client certificate required for all servlets */ ++ if (__pmServerHasFeature(PM_SERVER_FEATURE_CERT_REQD)) { ++#ifdef HAVE_OPENSSL ++ if (!client->stream.secure || ++ !client->secure.ssl || ++ SSL_get_peer_certificate(client->secure.ssl) == NULL) { ++ client->u.http.parser.status_code = HTTP_STATUS_FORBIDDEN; ++ } ++#else ++ /* no TLS support compiled in, reject all connections */ ++ client->u.http.parser.status_code = HTTP_STATUS_FORBIDDEN; ++#endif ++ } ++ + return sts; + } + diff --git a/pcp-7.1.5-scanmeta-LogLoadInDom-caller.patch b/pcp-7.1.5-scanmeta-LogLoadInDom-caller.patch new file mode 100644 index 0000000..44ab780 --- /dev/null +++ b/pcp-7.1.5-scanmeta-LogLoadInDom-caller.patch @@ -0,0 +1,43 @@ +From be9ade9950 Mon Sep 17 00:00:00 2001 +From: Nathan Scott +Subject: [PATCH] qa/src/scanmeta.c: fix call to __pmLogLoadInDom() + +scanmeta was *using* the acp == NULL guard to dodge the rlen test and +calling with rlen == 0 (this QA app was simply assuming the record +was valid, at least to the point that the buffer could be correctly +decoded). + +Fix involves re-extracting the correct record length and calling +__pmLogLoadInDom() with rlen != 0. +--- +diff --git a/qa/src/scanmeta.c b/qa/src/scanmeta.c +index aac953ae93..16c879c5fd 100644 +--- a/qa/src/scanmeta.c ++++ b/qa/src/scanmeta.c +@@ -143,7 +143,7 @@ free_elt_fields(elt_t *ep) + } + + void +-do_indom(__int32_t *buf, int type) ++do_indom(__int32_t *buf, int type, int len) + { + int sts; + static __pmTimestamp prior_stamp = { 0, 0 }; +@@ -156,7 +156,7 @@ do_indom(__int32_t *buf, int type) + elt_t *tp; + elt_t *dp = &dup; + +- if ((sts = __pmLogLoadInDom(NULL, 0, type, &lid, &buf)) < 0) { ++ if ((sts = __pmLogLoadInDom(NULL, len, type, &lid, &buf)) < 0) { + fprintf(stderr, "__pmLoadLoadInDom: failed: %s\n", pmErrStr(sts)); + return; + } +@@ -689,7 +689,7 @@ main(int argc, char *argv[]) + case TYPE_INDOM_V2: + if (!iflag) + break; +- do_indom(buf, hdr.type); ++ do_indom(buf, hdr.type, hdr.len); + break; + + case TYPE_LABEL: diff --git a/pcp-7.1.5-timezone-zoneinfo-validation.patch b/pcp-7.1.5-timezone-zoneinfo-validation.patch new file mode 100644 index 0000000..9b1d62a --- /dev/null +++ b/pcp-7.1.5-timezone-zoneinfo-validation.patch @@ -0,0 +1,297 @@ +From f86c0f4cda Mon Sep 17 00:00:00 2001 +From: Nathan Scott +Subject: [PATCH] libpcp, libpcp_web: validate timezone and zoneinfo strings + +The timezone and zoneinfo fields from archive labels and PDU_LOG_STATUS +are used via pmNewZone() -> setenv("TZ", ...), causing glibc to resolve +Olson timezone paths against /usr/share/zoneinfo/. A crafted value +like "../../etc/passwd" would cause glibc to open arbitrary files. + +Fix at two layers: +- Front door: add check_tz() check in pmLogGroupLabel() alongside the + existing check_hostname() check, rejecting unsafe timezone/zoneinfo + before any data is written to disk +- Consumption: add check_tz() check in pmNewZone() as defense-in-depth, + protecting against malicious archives created by other means + +The allowlist permits alphanumeric characters plus /_+-.:" which covers +both Olson paths (America/New_York) and POSIX TZ strings (EST5EDT). +Leading slashes and ".." path components are rejected. + +Add qa/src/check_tz.c and qa/2107 exercising pmNewZone() with valid +and malicious timezone strings. + +Co-Authored-By: Claude Opus 4.6 (1M context) +--- +diff --git a/qa/2107 b/qa/2107 +new file mode 100755 +index 0000000000..d4017c02fe +--- /dev/null ++++ b/qa/2107 +@@ -0,0 +1,60 @@ ++#!/bin/sh ++# PCP QA Test No. 2107 ++# Verify pmNewZone rejects unsafe timezone strings containing ++# path traversal or invalid characters ++# ++# Copyright (c) 2026 Red Hat. All Rights Reserved. ++# ++ ++seq=`basename $0` ++echo "QA output created by $seq" ++ ++# get standard environment, filters and checks ++. ./common.product ++. ./common.filter ++. ./common.check ++ ++_cleanup() ++{ ++ cd $here ++ $sudo rm -rf $tmp $tmp.* ++} ++ ++status=0 # success is the default! ++trap "_cleanup; exit \$status" 0 1 2 3 15 ++ ++# real QA test starts here ++ ++echo "=== valid Olson timezone ===" ++src/check_tz "America/New_York" ++ ++echo ++echo "=== valid POSIX timezone ===" ++src/check_tz "EST5EDT" ++ ++echo ++echo "=== valid simple timezone ===" ++src/check_tz "UTC" ++ ++echo ++echo "=== path traversal should be rejected ===" ++src/check_tz "../../etc/passwd" ++ ++echo ++echo "=== leading slash should be rejected ===" ++src/check_tz "/etc/localtime" ++ ++echo ++echo "=== semicolon should be rejected ===" ++src/check_tz "UTC;id" ++ ++echo ++echo "=== backtick should be rejected ===" ++src/check_tz 'UTC`id`' ++ ++echo ++echo "=== empty string should be accepted ===" ++src/check_tz "" ++ ++# success, all done ++exit +diff --git a/qa/2107.out b/qa/2107.out +new file mode 100644 +index 0000000000..42cf4d6adb +--- /dev/null ++++ b/qa/2107.out +@@ -0,0 +1,24 @@ ++QA output created by 2107 ++=== valid Olson timezone === ++pmNewZone("America/New_York") -> 0 (accepted) ++ ++=== valid POSIX timezone === ++pmNewZone("EST5EDT") -> 0 (accepted) ++ ++=== valid simple timezone === ++pmNewZone("UTC") -> 0 (accepted) ++ ++=== path traversal should be rejected === ++pmNewZone("../../etc/passwd") -> Invalid argument (rejected) ++ ++=== leading slash should be rejected === ++pmNewZone("/etc/localtime") -> Invalid argument (rejected) ++ ++=== semicolon should be rejected === ++pmNewZone("UTC;id") -> Invalid argument (rejected) ++ ++=== backtick should be rejected === ++pmNewZone("UTC`id`") -> Invalid argument (rejected) ++ ++=== empty string should be accepted === ++pmNewZone("") - skipped (empty string) +diff --git a/qa/group b/qa/group +index 16c5aa35e3..1103b1f526 100644 +--- a/qa/group ++++ b/qa/group +@@ -2378,5 +2378,6 @@ suse + 2101 pmda.sockets local security + 2102 pmlogmv local security + 2103 pmieconf local security ++2107 libpcp local security + 4751 libpcp threads valgrind local pcp helgrind + 9000 other local +diff --git a/qa/src/GNUlocaldefs b/qa/src/GNUlocaldefs +index 2009bd65b0..1d2fa9982b 100644 +--- a/qa/src/GNUlocaldefs ++++ b/qa/src/GNUlocaldefs +@@ -63,7 +63,7 @@ CFILES = disk_test.c exercise.c context_test.c chkoptfetch.c \ + multithread8.c multithread9.c multithread10.c multithread11.c \ + multithread12.c multithread13.c multithread14.c \ + exerlock.c hashwalk.c parsehostattrs.c parsehostspec.c getoptions.c \ +- check_cloexec.c ++ check_cloexec.c check_tz.c + + + ifeq ($(shell test -f ../localconfig && echo 1), 1) +@@ -1399,6 +1399,11 @@ check_cloexec: check_cloexec.c + $(CCF) $(CDEFS) -o $@ $@.c $(LDLIBS) + $(LINKER_MAKERULE) + ++check_tz: check_tz.c ++ rm -f $@ ++ $(CCF) $(CDEFS) -o $@ $@.c $(LDLIBS) ++ $(LINKER_MAKERULE) ++ + check_import: check_import.c + rm -f $@ + $(CCF) $(CDEFS) -o $@ $@.c $(LDLIBS) -lpcp_archive -lpcp_import +diff --git a/qa/src/check_tz.c b/qa/src/check_tz.c +new file mode 100644 +index 0000000000..ad96444f9c +--- /dev/null ++++ b/qa/src/check_tz.c +@@ -0,0 +1,31 @@ ++/* ++ * Verify pmNewZone accepts/rejects timezone strings correctly. ++ */ ++ ++#include ++ ++int ++main(int argc, char **argv) ++{ ++ int sts; ++ ++ pmSetProgname(argv[0]); ++ ++ if (argc != 2) { ++ fprintf(stderr, "Usage: %s timezone\n", pmGetProgname()); ++ return 1; ++ } ++ ++ if (argv[1][0] == '\0') { ++ printf("pmNewZone(\"\") - skipped (empty string)\n"); ++ return 0; ++ } ++ ++ sts = pmNewZone(argv[1]); ++ if (sts >= 0) ++ printf("pmNewZone(\"%s\") -> %d (accepted)\n", argv[1], sts); ++ else ++ printf("pmNewZone(\"%s\") -> %s (rejected)\n", argv[1], pmErrStr(sts)); ++ ++ return 0; ++} +diff --git a/src/libpcp/src/tz.c b/src/libpcp/src/tz.c +index e8f1d80cd2..7073b12fe9 100644 +--- a/src/libpcp/src/tz.c ++++ b/src/libpcp/src/tz.c +@@ -25,6 +25,7 @@ + * lock initialization in pmNewContext(). + */ + ++#include + #include "pmapi.h" + #include "libpcp.h" + #include "sha256.h" +@@ -570,6 +571,22 @@ pmUseZone(const int tz_handle) + return 0; + } + ++static int ++valid_tz(const char *tz) ++{ ++ const char *p; ++ ++ if (tz == NULL || tz[0] == '\0' || tz[0] == '/') ++ return 0; ++ for (p = tz; *p; p++) { ++ if (!isalnum((unsigned char)*p) && strchr("/_+-.:,\"'", *p) == NULL) ++ return 0; ++ } ++ if (strstr(tz, "..") != NULL) ++ return 0; ++ return 1; ++} ++ + int + pmNewZone(const char *tz) + { +@@ -577,6 +594,13 @@ pmNewZone(const char *tz) + int hack = 0; + int sts; + ++ if (!valid_tz(tz)) { ++ if (pmDebugOptions.context) ++ fprintf(stderr, "%s: rejecting unsafe timezone: %s\n", ++ __FUNCTION__, tz ? tz : "(null)"); ++ return -EINVAL; ++ } ++ + PM_LOCK(__pmLock_extcall); + + len = (int)strlen(tz); +diff --git a/src/libpcp_web/src/loggroup.c b/src/libpcp_web/src/loggroup.c +index cef3a8d2c5..ca673b54fb 100644 +--- a/src/libpcp_web/src/loggroup.c ++++ b/src/libpcp_web/src/loggroup.c +@@ -601,6 +601,30 @@ check_hostname(const char *hostname) + return 1; + } + ++/* ++ * Check that timezone/zoneinfo strings (similarly can arrive from a ++ * remote host), conform to simple validity checks; for timezone the ++ * string will be placed into the environment (TZ), but for zoneinfo ++ * file system path lookup will occur when accessing Olsen database. ++ */ ++static int ++check_tz(const char *tz) ++{ ++ const char *p; ++ ++ if (tz == NULL || tz[0] == '\0') ++ return 1; /* empty/NULL timezone is valid (use system default) */ ++ if (tz[0] == '/') ++ return 0; ++ for (p = tz; *p; p++) { ++ if (!isalnum((unsigned char)*p) && strchr("/_+-.:,", *p) == NULL) ++ return 0; ++ } ++ if (strstr(tz, "..") != NULL) ++ return 0; ++ return 1; ++} ++ + int + pmLogGroupLabel(pmLogGroupSettings *sp, const char *content, size_t length, + dict *params, void *arg) +@@ -636,6 +660,18 @@ pmLogGroupLabel(pmLogGroupSettings *sp, const char *content, size_t length, + sts = -EINVAL; + goto fail; + } ++ if (!check_tz(loglabel.timezone)) { ++ pmNotifyErr(LOG_ERR, "Rejecting archive with unsafe timezone: %s", ++ loglabel.timezone ? loglabel.timezone : "(null)"); ++ sts = -EINVAL; ++ goto fail; ++ } ++ if (!check_tz(loglabel.zoneinfo)) { ++ pmNotifyErr(LOG_ERR, "Rejecting archive with unsafe zoneinfo: %s", ++ loglabel.zoneinfo ? loglabel.zoneinfo : "(null)"); ++ sts = -EINVAL; ++ goto fail; ++ } + + start = (time_t)loglabel.start.sec; + if (localtime_r(&start, &tm) == NULL || diff --git a/pcp.spec b/pcp.spec index bc6e766..b8e9ede 100644 --- a/pcp.spec +++ b/pcp.spec @@ -1,6 +1,6 @@ Name: pcp Version: 7.1.5 -Release: 5%{?dist} +Release: 6%{?dist} Summary: System-level performance monitoring and performance management License: GPL-2.0-or-later AND LGPL-2.1-or-later AND CC-BY-3.0 URL: https://pcp.io @@ -35,6 +35,48 @@ Patch4: pcp-7.1.5-CVE-2026-16526.patch # https://github.com/performancecopilot/pcp/commit/c5cbeceb7d3c2af357c04065cdd911efdc270de0 Patch5: pcp-7.1.5-CVE-2026-16524.patch +# https://github.com/performancecopilot/pcp/commit/dc73ec0f579028620e408114427d27ad0afd24a8 +Patch6: pcp-7.1.5-pmlogmv-command-injection.patch + +# https://github.com/performancecopilot/pcp/commit/cdc9676ab60176476a23f323c145d8d0fc71e165 +Patch7: pcp-7.1.5-pmieconf-command-injection.patch + +# https://github.com/performancecopilot/pcp/commit/ccd1bb167934b5e4a60d39bacf25e12e0a2decb4 +Patch8: pcp-7.1.5-OOB-pmLogLoadLabelSet.patch + +# https://github.com/performancecopilot/pcp/commit/7f42013d33a6717a45e8f9ac9e7dfcf76ee6384f +Patch9: pcp-7.1.5-OOB-pmDiscoverDecodeMetaInDom.patch + +# https://github.com/performancecopilot/pcp/commit/e512482e7d33aa6c46fbe569d0de7951c0fcec75 +Patch10: pcp-7.1.5-OOB-pmDecodeLabel.patch + +# https://github.com/performancecopilot/pcp/commit/5366a546d66c0a085958dd84b08b3e1ca165f7ab +Patch11: pcp-7.1.5-OOB-pmDecodeLogStatus.patch + +# https://github.com/performancecopilot/pcp/commit/b743fc58795bd3667b78e3b0759251fef27a983b +Patch12: pcp-7.1.5-OOB-pmDecodeInstance.patch + +# https://github.com/performancecopilot/pcp/commit/7465d7cbdb2025ea8666b621a03b958f7e15f0dc +Patch13: pcp-7.1.5-pducrash-oob-tests.patch + +# https://github.com/performancecopilot/pcp/commit/f86c0f4cdac8df0b6fb05f60872f6f29bbc96429 +Patch14: pcp-7.1.5-timezone-zoneinfo-validation.patch + +# https://github.com/performancecopilot/pcp/commit/2a4bd9f81a3a59fcba4cf726cc615fd51d107054 +Patch15: pcp-7.1.5-pmdaroot-peer-credentials.patch + +# https://github.com/performancecopilot/pcp/commit/81a9efe96db6f7764a1a1646b64eb85de5cdaf77 +Patch16: pcp-7.1.5-pmproxy-rest-certreqd.patch + +# https://github.com/performancecopilot/pcp/commit/4121ae06f54ab2089adf7270197c774d60dd03c8 +Patch17: pcp-7.1.5-pmproxy-logger-auth.patch + +# https://github.com/performancecopilot/pcp/commit/bf898b50ef2c51ac730252a6e0ef112964083aa9 +Patch18: pcp-7.1.5-pmproxy-logger-meta-network.patch + +# https://github.com/performancecopilot/pcp/commit/be9ade995083a2732741789199209e2eb01b9fbd +Patch19: pcp-7.1.5-scanmeta-LogLoadInDom-caller.patch + # The additional linker flags break out-of-tree PMDAs. # https://bugzilla.redhat.com/show_bug.cgi?id=2043092 %undefine _package_note_flags @@ -3448,6 +3490,14 @@ fi %files zeroconf -f pcp-zeroconf-files.rpm %changelog +* Wed Aug 12 2026 Jan Kuřík - 7.1.5-6 +- Backport remaining PCP security hardening fixes from private-pcp +- pmieconf and pmlogmv command injection hardening (CWE-78) +- libpcp PDU decode OOB read and overflow guards (CWE-125/190/195) +- timezone and zoneinfo string validation +- pmdaroot peer credential verification (CWE-403) +- pmproxy REST CERT_REQD enforcement and logger servlet authentication + * Wed Aug 12 2026 Jan Kuřík - 7.1.5-5 - Fix CVE-2026-16524: linux_sockets PMDA command injection (RHEL-213661) - Fix CVE-2026-16526: FD_CLOEXEC privilege escalation via pmdaroot (RHEL-213692)