Backport remaining private-pcp security hardening fixes for 7.1.5-6
Add patches for pmieconf/pmlogmv command injection, libpcp PDU decode OOB guards, timezone validation, pmdaroot peer credentials, and pmproxy REST CERT_REQD and logger authentication hardening. Resolves: RHEL-213756 CVE-2026-16531 Resolves: RHEL-213746 CVE-2026-16530 Resolves: RHEL-213726 CVE-2026-16529 Resolves: RHEL-213717 CVE-2026-16527 Resolves: RHEL-213692 CVE-2026-16526 Resolves: RHEL-213661 CVE-2026-16524 Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
54531fc403
commit
4d55c1806b
58
pcp-7.1.5-OOB-pmDecodeInstance.patch
Normal file
58
pcp-7.1.5-OOB-pmDecodeInstance.patch
Normal file
@ -0,0 +1,58 @@
|
||||
From b743fc5879 Mon Sep 17 00:00:00 2001
|
||||
From: Nathan Scott <nathans@redhat.com>
|
||||
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) <noreply@anthropic.com>
|
||||
---
|
||||
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",
|
||||
57
pcp-7.1.5-OOB-pmDecodeLabel.patch
Normal file
57
pcp-7.1.5-OOB-pmDecodeLabel.patch
Normal file
@ -0,0 +1,57 @@
|
||||
From e512482e7d Mon Sep 17 00:00:00 2001
|
||||
From: Nathan Scott <nathans@redhat.com>
|
||||
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) <noreply@anthropic.com>
|
||||
---
|
||||
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;
|
||||
}
|
||||
472
pcp-7.1.5-OOB-pmDecodeLogStatus.patch
Normal file
472
pcp-7.1.5-OOB-pmDecodeLogStatus.patch
Normal file
@ -0,0 +1,472 @@
|
||||
From 5366a546d6 Mon Sep 17 00:00:00 2001
|
||||
From: Nathan Scott <nathans@redhat.com>
|
||||
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) <noreply@anthropic.com>
|
||||
---
|
||||
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) {
|
||||
37
pcp-7.1.5-OOB-pmDiscoverDecodeMetaInDom.patch
Normal file
37
pcp-7.1.5-OOB-pmDiscoverDecodeMetaInDom.patch
Normal file
@ -0,0 +1,37 @@
|
||||
From 7f42013d33 Mon Sep 17 00:00:00 2001
|
||||
From: Nathan Scott <nathans@redhat.com>
|
||||
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) <noreply@anthropic.com>
|
||||
---
|
||||
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);
|
||||
102
pcp-7.1.5-OOB-pmLogLoadLabelSet.patch
Normal file
102
pcp-7.1.5-OOB-pmLogLoadLabelSet.patch
Normal file
@ -0,0 +1,102 @@
|
||||
From ccd1bb1679 Mon Sep 17 00:00:00 2001
|
||||
From: Nathan Scott <nathans@redhat.com>
|
||||
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) <noreply@anthropic.com>
|
||||
---
|
||||
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);
|
||||
157
pcp-7.1.5-pducrash-oob-tests.patch
Normal file
157
pcp-7.1.5-pducrash-oob-tests.patch
Normal file
@ -0,0 +1,157 @@
|
||||
From 7465d7cbdb Mon Sep 17 00:00:00 2001
|
||||
From: Nathan Scott <nathans@redhat.com>
|
||||
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) <noreply@anthropic.com>
|
||||
---
|
||||
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
|
||||
112
pcp-7.1.5-pmdaroot-peer-credentials.patch
Normal file
112
pcp-7.1.5-pmdaroot-peer-credentials.patch
Normal file
@ -0,0 +1,112 @@
|
||||
From 2a4bd9f81a Mon Sep 17 00:00:00 2001
|
||||
From: Nathan Scott <nathans@redhat.com>
|
||||
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) <noreply@anthropic.com>
|
||||
---
|
||||
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 <pwd.h>
|
||||
+#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);
|
||||
}
|
||||
142
pcp-7.1.5-pmieconf-command-injection.patch
Normal file
142
pcp-7.1.5-pmieconf-command-injection.patch
Normal file
@ -0,0 +1,142 @@
|
||||
From cdc9676ab6 Mon Sep 17 00:00:00 2001
|
||||
From: Nathan Scott <nathans@redhat.com>
|
||||
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) <noreply@anthropic.com>
|
||||
---
|
||||
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;
|
||||
}
|
||||
}
|
||||
549
pcp-7.1.5-pmlogmv-command-injection.patch
Normal file
549
pcp-7.1.5-pmlogmv-command-injection.patch
Normal file
@ -0,0 +1,549 @@
|
||||
From dc73ec0f57 Mon Sep 17 00:00:00 2001
|
||||
From: Nathan Scott <nathans@redhat.com>
|
||||
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 <file") with __pmProcessPipe() which uses
|
||||
execvp() internally, passing filenames as argv not shell words
|
||||
- Expand check_name() blocklist to include backtick, braces, backslash,
|
||||
bang, newline and tab (defense-in-depth, no longer the security
|
||||
boundary)
|
||||
- Apply check_name() to source names as well as destination names
|
||||
- Add qa/2102 verifying metacharacter rejection and normal copy
|
||||
|
||||
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) <noreply@anthropic.com>
|
||||
---
|
||||
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 <unistd.h>
|
||||
@@ -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 */
|
||||
206
pcp-7.1.5-pmproxy-logger-auth.patch
Normal file
206
pcp-7.1.5-pmproxy-logger-auth.patch
Normal file
@ -0,0 +1,206 @@
|
||||
From 4121ae06f5 Mon Sep 17 00:00:00 2001
|
||||
From: Nathan Scott <nathans@redhat.com>
|
||||
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) <noreply@anthropic.com>
|
||||
---
|
||||
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 <<EOF
|
||||
+[pmproxy]
|
||||
+pcp.enabled = true
|
||||
+http.enabled = true
|
||||
+[pmlogger]
|
||||
+enabled = true
|
||||
+authenticate = true
|
||||
+EOF
|
||||
+$PCP_BINADM_DIR/pmproxy -f -p $__port -l $tmp.log -c $tmp.conf &
|
||||
+__pid=$!
|
||||
+sleep 1
|
||||
+if ! kill -0 $__pid 2>/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 <<EOF
|
||||
+[pmproxy]
|
||||
+pcp.enabled = true
|
||||
+http.enabled = true
|
||||
+[pmlogger]
|
||||
+enabled = true
|
||||
+EOF
|
||||
+$PCP_BINADM_DIR/pmproxy -f -p $__port -l $tmp.log2 -c $tmp.conf2 &
|
||||
+__pid=$!
|
||||
+sleep 1
|
||||
+if ! kill -0 $__pid 2>/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");
|
||||
|
||||
183
pcp-7.1.5-pmproxy-logger-meta-network.patch
Normal file
183
pcp-7.1.5-pmproxy-logger-meta-network.patch
Normal file
@ -0,0 +1,183 @@
|
||||
From bf898b50ef Mon Sep 17 00:00:00 2001
|
||||
From: Nathan Scott <nathans@redhat.com>
|
||||
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) <noreply@anthropic.com>
|
||||
---
|
||||
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 <<EOF
|
||||
+[pmproxy]
|
||||
+pcp.enabled = true
|
||||
+http.enabled = true
|
||||
+[discover]
|
||||
+enabled = false
|
||||
+EOF
|
||||
+PCP_REMOTE_ARCHIVE_DIR=$tmp.archdir $PCP_BINADM_DIR/pmproxy -f -p $__port -l $tmp.log -c $tmp.conf &
|
||||
+__pid=$!
|
||||
+sleep 2
|
||||
+
|
||||
+if ! kill -0 $__pid 2>/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 ===
|
||||
42
pcp-7.1.5-pmproxy-rest-certreqd.patch
Normal file
42
pcp-7.1.5-pmproxy-rest-certreqd.patch
Normal file
@ -0,0 +1,42 @@
|
||||
From 81a9efe96d Mon Sep 17 00:00:00 2001
|
||||
From: Nathan Scott <nathans@redhat.com>
|
||||
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) <noreply@anthropic.com>
|
||||
---
|
||||
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;
|
||||
}
|
||||
|
||||
43
pcp-7.1.5-scanmeta-LogLoadInDom-caller.patch
Normal file
43
pcp-7.1.5-scanmeta-LogLoadInDom-caller.patch
Normal file
@ -0,0 +1,43 @@
|
||||
From be9ade9950 Mon Sep 17 00:00:00 2001
|
||||
From: Nathan Scott <nathans@redhat.com>
|
||||
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:
|
||||
297
pcp-7.1.5-timezone-zoneinfo-validation.patch
Normal file
297
pcp-7.1.5-timezone-zoneinfo-validation.patch
Normal file
@ -0,0 +1,297 @@
|
||||
From f86c0f4cda Mon Sep 17 00:00:00 2001
|
||||
From: Nathan Scott <nathans@redhat.com>
|
||||
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) <noreply@anthropic.com>
|
||||
---
|
||||
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 <pcp/pmapi.h>
|
||||
+
|
||||
+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 <ctype.h>
|
||||
#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 ||
|
||||
52
pcp.spec
52
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 <jkurik@redhat.com> - 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 <jkurik@redhat.com> - 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)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user