rpms/pcp
7
0
Fork 0
Code Issues Pull Requests Releases Activity
c9s
pcp/pcp-RHEL-213747.patch
RHEL Packaging Agent 4f4f4d25c8 Fix CVE-2026-16530: arbitrary pointer deref in __pmLogLoadInDom 
Backport fix for CVE-2026-16530 (CWE-125/822) - an arbitrary
pointer dereference in __pmLogLoadInDom. The bounds check on
string indices was guarded by acp!=NULL, making it unreachable
from the streaming path used by pmproxy. Three upstream commits
are backported: the primary fix adding minimum rlen validation
and unconditional bounds checks, plus two follow-up fixes for
pmlogrewrite and pmlogextract that relied on the removed guard.

CVE: CVE-2026-16530
Upstream patches:
 - ec81e2b35c.patch
 - b72da5135d.patch
 - edfd909eda.patch
Resolves: RHEL-213747

This commit was backported by Ymir, a Red Hat Enterprise Linux software maintenance AI agent.

Assisted-by: Ymir
2026-07-30 13:05:07 +00:00

318 lines
12 KiB
Diff
Raw Permalink Blame History

From 1f232730588b2f0bf8c6777bb478635aba6c6dc5 Mon Sep 17 00:00:00 2001
From: Nathan Scott <nathans@redhat.com>
Date: Thu, 2 Jul 2026 15:10:16 +1000
Subject: [PATCH 1/3] libpcp: fix arbitrary pointer deref in __pmLogLoadInDom
(CWE-125/822)
The bounds check on string indices (idx > max_idx) in __pmLogLoadInDom()
was guarded by if (acp != NULL), making it unreachable from the streaming
path used by pmproxy (which passes acp=NULL). An attacker could submit
a TYPE_INDOM record with an out-of-range stridx value via POST
/logger/meta, causing namelist[i] to point to arbitrary heap memory.
Fix:
- Add minimum rlen checks before reading fixed fields, using macros
derived from the on-disk struct sizes (INDOM_V3_MINRLEN, INDOM_V2_MINRLEN)
- Validate numinst against rlen before using it in arithmetic, preventing
integer overflow in the max_idx computation
- Make max_idx computation and idx bounds check unconditional (remove the
acp != NULL guard) so they protect both archive and streaming paths
- Extend qa/src/pducrash.c with decode_log_indom() exercising all four
failure modes via __pmLogLoadInDom(NULL, ...)
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>
---
qa/513.out | 8 +++++
qa/src/pducrash.c | 77 ++++++++++++++++++++++++++++++++++++++++
src/libpcp/src/e_indom.c | 63 ++++++++++++++++++++------------
3 files changed, 126 insertions(+), 22 deletions(-)
diff --git a/qa/513.out b/qa/513.out
index 5894ca159..7ccaa1f37 100644
--- a/qa/513.out
+++ b/qa/513.out
@@ -269,6 +269,14 @@ QA output created by 513
__pmDecodeDescs: sts = -12366 (IPC protocol failure)
[descs] checking access beyond extended buffer
__pmDecodeDescs: sts = -12366 (IPC protocol failure)
+[log_indom] checking rlen too small for v3 header
+ __pmLogLoadInDom: sts = -12373 (Corrupted record in a PCP archive)
+[log_indom] checking rlen too small for v2 header
+ __pmLogLoadInDom: sts = -12373 (Corrupted record in a PCP archive)
+[log_indom] checking numinst larger than rlen allows
+ __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)
=== filtered valgrind report ===
Memcheck, a memory error detector
Command: src/pducrash
diff --git a/qa/src/pducrash.c b/qa/src/pducrash.c
index 26dbd2414..76e1e0ce7 100644
--- a/qa/src/pducrash.c
+++ b/qa/src/pducrash.c
@@ -1627,6 +1627,82 @@ decode_trace_data(const char *name)
free(trace_data);
}
+/*
+ * Test __pmLogLoadInDom with acp==NULL (streaming path used by pmproxy).
+ * The on-disk v3 record layout (after len+type header) is:
+ * sec[2], nsec, indom, numinst, instlist[numinst],
+ * stridx[numinst], name_strings
+ * rlen is the body length excluding the 2-word header.
+ */
+static void
+decode_log_indom(const char *name)
+{
+ int sts;
+ __pmLogInDom lid;
+ __int32_t *buf;
+
+ /* TYPE_INDOM (v3): rlen too small for fixed header fields */
+ fprintf(stderr, "[%s] checking rlen too small for v3 header\n", name);
+ {
+ __int32_t tiny[1];
+ memset(&lid, 0, sizeof(lid));
+ memset(tiny, 0, sizeof(tiny));
+ buf = tiny;
+ sts = __pmLogLoadInDom(NULL, 4, TYPE_INDOM, &lid, &buf);
+ fprintf(stderr, " __pmLogLoadInDom: sts = %d (%s)\n", sts, pmErrStr(sts));
+ }
+
+ /* TYPE_INDOM_V2: rlen too small for fixed header fields */
+ fprintf(stderr, "[%s] checking rlen too small for v2 header\n", name);
+ {
+ __int32_t tiny[1];
+ memset(&lid, 0, sizeof(lid));
+ memset(tiny, 0, sizeof(tiny));
+ buf = tiny;
+ sts = __pmLogLoadInDom(NULL, 4, TYPE_INDOM_V2, &lid, &buf);
+ fprintf(stderr, " __pmLogLoadInDom: sts = %d (%s)\n", sts, pmErrStr(sts));
+ }
+
+ /* TYPE_INDOM (v3): numinst too large for rlen */
+ fprintf(stderr, "[%s] checking numinst larger than rlen allows\n", name);
+ {
+ /* v3 fixed fields: sec[2]+nsec+indom+numinst = 5 words (20 bytes) */
+ __int32_t rec[7]; /* room for header + fixed fields */
+ memset(&lid, 0, sizeof(lid));
+ memset(rec, 0, sizeof(rec));
+ rec[0] = htonl(sizeof(rec)); /* len (not used but for completeness) */
+ rec[1] = htonl(TYPE_INDOM); /* type */
+ /* sec[2], nsec, indom are zero */
+ rec[6] = htonl(999999); /* numinst - way too large */
+ buf = &rec[2]; /* skip len+type, as __pmLogLoadInDom expects */
+ sts = __pmLogLoadInDom(NULL, 20, TYPE_INDOM, &lid, &buf);
+ fprintf(stderr, " __pmLogLoadInDom: sts = %d (%s)\n", sts, pmErrStr(sts));
+ }
+
+ /* TYPE_INDOM (v3): valid numinst=1 but stridx out of range */
+ fprintf(stderr, "[%s] checking out-of-range stridx with acp==NULL\n", name);
+ {
+ /*
+ * Layout after len+type: sec[2], nsec, indom, numinst,
+ * instlist[1], stridx[1], (no string data)
+ * That's 5 + 1 + 1 = 7 words = 28 bytes of body.
+ */
+ __int32_t rec[9]; /* 2 (header) + 7 (body) */
+ memset(&lid, 0, sizeof(lid));
+ memset(rec, 0, sizeof(rec));
+ rec[0] = htonl(sizeof(rec)); /* len */
+ rec[1] = htonl(TYPE_INDOM); /* type */
+ /* sec[2], nsec, indom are zero */
+ rec[6] = htonl(1); /* numinst */
+ rec[7] = htonl(0); /* instlist[0] = 0 */
+ rec[8] = htonl(0x7FFFFFFF); /* stridx[0] = huge OOB index */
+ buf = &rec[2];
+ sts = __pmLogLoadInDom(NULL, 28, TYPE_INDOM, &lid, &buf);
+ fprintf(stderr, " __pmLogLoadInDom: sts = %d (%s)\n", sts, pmErrStr(sts));
+ if (sts >= 0) __pmFreeLogInDom(&lid);
+ }
+}
+
typedef void (*decode_t)(const char *);
struct pdu {
@@ -1661,6 +1737,7 @@ struct pdu {
{ "highres_result", decode_highres_result },
{ "desc_ids", decode_desc_ids },
{ "descs", decode_descs },
+ { "log_indom", decode_log_indom },
};
int
diff --git a/src/libpcp/src/e_indom.c b/src/libpcp/src/e_indom.c
index 8036fc6cc..de44751f0 100644
--- a/src/libpcp/src/e_indom.c
+++ b/src/libpcp/src/e_indom.c
@@ -51,6 +51,10 @@ typedef struct {
/* will be expanded if numinst > 0 */
} __pmInDom_v2;
+/* Minimum rlen (record body without len+type header) to read fixed fields */
+#define INDOM_V3_MINRLEN (sizeof(__pmInDom_v3) - 2 * sizeof(__int32_t))
+#define INDOM_V2_MINRLEN (sizeof(__pmInDom_v2) - 2 * sizeof(__int32_t))
+
/*
* pack an indom into a physical metadata record
* - lcp required to provide archive version (else NULL)
@@ -258,33 +262,55 @@ PM_FAULT_POINT("libpcp/" __FILE__ ":3", PM_FAULT_ALLOC);
if (type == TYPE_INDOM || type == TYPE_INDOM_DELTA) {
__pmInDom_v3 *v3;
+ if (rlen < (int)INDOM_V3_MINRLEN) {
+ if (pmDebugOptions.logmeta)
+ fprintf(stderr, "__pmLogLoadInDom: v3 rlen=%d too small (min=%d)\n",
+ rlen, (int)INDOM_V3_MINRLEN);
+ goto bad;
+ }
v3 = (__pmInDom_v3 *)&lbuf[-2]; /* len+type not in buf */
__pmLoadTimestamp(&v3->sec[0], &lidp->stamp);
k = (sizeof(v3->sec)+sizeof(v3->nsec))/sizeof(__int32_t);
lidp->indom = __ntohpmInDom(v3->indom);
k++;
lidp->numinst = ntohl(v3->numinst);
+ if (lidp->numinst < 0 ||
+ lidp->numinst > (rlen - (int)INDOM_V3_MINRLEN) / (2 * (int)sizeof(__int32_t))) {
+ if (pmDebugOptions.logmeta)
+ fprintf(stderr, "__pmLogLoadInDom: v3 numinst=%d not consistent with rlen=%d\n",
+ lidp->numinst, rlen);
+ goto bad;
+ }
k++;
lidp->instlist = (int *)&v3->data;
- if (acp != NULL) {
- /* rlen minus fixed fields (plus len+type), minus instlist[], minus strindex[] */
- max_idx = rlen - 5*sizeof(__int32_t) - 2*lidp->numinst*sizeof(__int32_t);
- }
+ /* rlen minus fixed fields (plus len+type), minus instlist[], minus strindex[] */
+ max_idx = rlen - (int)INDOM_V3_MINRLEN - 2 * lidp->numinst * (int)sizeof(__int32_t);
}
else if (type == TYPE_INDOM_V2) {
__pmInDom_v2 *v2;
+ if (rlen < (int)INDOM_V2_MINRLEN) {
+ if (pmDebugOptions.logmeta)
+ fprintf(stderr, "__pmLogLoadInDom: v2 rlen=%d too small (min=%d)\n",
+ rlen, (int)INDOM_V2_MINRLEN);
+ goto bad;
+ }
v2 = (__pmInDom_v2 *)&lbuf[-2]; /* len+type not in lbuf */
__pmLoadTimeval(&v2->sec, &lidp->stamp);
k = (sizeof(v2->sec)+sizeof(v2->usec))/sizeof(__int32_t);
lidp->indom = __ntohpmInDom(v2->indom);
k++;
lidp->numinst = ntohl(v2->numinst);
+ if (lidp->numinst < 0 ||
+ lidp->numinst > (rlen - (int)INDOM_V2_MINRLEN) / (2 * (int)sizeof(__int32_t))) {
+ if (pmDebugOptions.logmeta)
+ fprintf(stderr, "__pmLogLoadInDom: v2 numinst=%d not consistent with rlen=%d\n",
+ lidp->numinst, rlen);
+ goto bad;
+ }
k++;
lidp->instlist = (int *)&v2->data;
- if (acp != NULL) {
- /* rlen minus fixed fields (plus len+type), minus instlist[], minus strindex[] */
- max_idx = rlen - 4*sizeof(__int32_t) - 2*lidp->numinst*sizeof(__int32_t);
- }
+ /* rlen minus fixed fields (plus len+type), minus instlist[], minus strindex[] */
+ max_idx = rlen - (int)INDOM_V2_MINRLEN - 2 * lidp->numinst * (int)sizeof(__int32_t);
}
else {
if (pmDebugOptions.logmeta)
@@ -327,21 +353,14 @@ PM_FAULT_POINT("libpcp/" __FILE__ ":4", PM_FAULT_ALLOC);
}
idx = ntohl(stridx[i]);
if (idx >= 0) {
- if (acp != NULL) {
- /*
- * crude sanity check ... if the index points to the
- * start of the name that is past the end of the input
- * record, the record is corrupted
- */
- if (idx > max_idx) {
- if (pmDebugOptions.logmeta) {
- char strbuf[20];
- fprintf(stderr, "__pmLogLoadInDom: InDom: %s instance[%d]: bad string index (%d) > max index based on record length (%d)\n",
- pmInDomStr_r(lidp->indom, strbuf, sizeof(strbuf)),
- i, idx, max_idx);
- }
- goto bad;
+ if (idx > max_idx) {
+ if (pmDebugOptions.logmeta) {
+ char strbuf[20];
+ fprintf(stderr, "__pmLogLoadInDom: InDom: %s instance[%d]: bad string index (%d) > max index based on record length (%d)\n",
+ pmInDomStr_r(lidp->indom, strbuf, sizeof(strbuf)),
+ i, idx, max_idx);
}
+ goto bad;
}
lidp->namelist[i] = &namebase[idx];
if (pmDebugOptions.logmeta && pmDebugOptions.desperate)
From d16c0cbdc680a539a64433716d6888003e086c81 Mon Sep 17 00:00:00 2001
From: Ken McDonell <kenj@kenj.id.au>
Date: Sat, 11 Jul 2026 07:29:38 +1000
Subject: [PATCH 2/3] src/pmlogrewrite/indom.c: fix call to __pmLogLoadInDom()
Turns out pmlogrewrite was *using* the acp == NULL guard to dodge the
rlen test and calling with rlen == 0 (this was correct as the *same* record
had previously been processed elsewhere with the correct rlen so the
buffer was know to be good).
Fix involves re-extracting the correct record length and calling
__pmLogLoadInDom() with rlen != 0.
---
src/pmlogrewrite/indom.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/pmlogrewrite/indom.c b/src/pmlogrewrite/indom.c
index eeef646fc..0300aa9a2 100644
--- a/src/pmlogrewrite/indom.c
+++ b/src/pmlogrewrite/indom.c
@@ -223,9 +223,10 @@ _pmUnpackInDom(__int32_t *recbuf, __pmLogInDom *lidp)
}
else {
__int32_t *buf;
+ int len = htonl(hdr->len);
/* buffer for __pmLogLoadInDom has to start AFTER the header */
buf = &recbuf[2];
- sts = __pmLogLoadInDom(NULL, 0, type, lidp, &buf);
+ sts = __pmLogLoadInDom(NULL, len, type, lidp, &buf);
if (sts < 0) {
fprintf(stderr, "_pmUnpackInDom: __pmLogLoadInDom(type=%d): failed: %s\n", type, pmErrStr(sts));
abandon();
From b07e33d85a93a66b63e2c4e579100d0ae999a622 Mon Sep 17 00:00:00 2001
From: Ken McDonell <kenj@kenj.id.au>
Date: Sat, 11 Jul 2026 08:12:58 +1000
Subject: [PATCH 3/3] src/pmlogextract/pmlogextract.c: fix call to
__pmLogLoadInDom()
pmlogextract was also *using* the acp == NULL guard to dodge the rlen
test and calling with rlen == 0.
Fix involves using the correct record length and calling
__pmLogLoadInDom() with rlen != 0.
---
src/pmlogextract/pmlogextract.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/pmlogextract/pmlogextract.c b/src/pmlogextract/pmlogextract.c
index 93a3d3988..eac7bf610 100644
--- a/src/pmlogextract/pmlogextract.c
+++ b/src/pmlogextract/pmlogextract.c
@@ -1300,7 +1300,7 @@ write_rec(reclist_t *rec)
memcpy(buf, rec->pdu, rlen);
ibuf = &buf[2];
- sts = __pmLogLoadInDom(NULL, 0, type, &lid, &ibuf);
+ sts = __pmLogLoadInDom(NULL, rlen, type, &lid, &ibuf);
if (sts < 0) {
fprintf(stderr, "write_rec: __pmLogLoadInDom(type=%s (%d)): failed: %s\n", __pmLogMetaTypeStr(type), type, pmErrStr(sts));
}
Reference in New Issue View Git Blame Copy Permalink