From eed8605d0a40ba5782207d416e01e382c860db7e Mon Sep 17 00:00:00 2001 From: Jan Kurik Date: Fri, 24 Jul 2026 06:59:10 +0200 Subject: [PATCH 1/2] libpcp: reject undersized metadata records in __pmLogLoadMeta __pmLogLoadMeta() rejects undersized metadata records qa/2009: exercise __pmLogLoadMeta undersized metadata record rejection Co-authored-by: Cursor --- qa/2009 | 85 +++++++++++++++++++++++++++++++++++++++ qa/2009.out | 41 +++++++++++++++++++ qa/group | 1 + src/libpcp/src/logmeta.c | 9 ++++- src/libpcp3/src/logmeta.c | 9 ++++- 5 files changed, 143 insertions(+), 2 deletions(-) create mode 100755 qa/2009 create mode 100644 qa/2009.out diff --git a/qa/2009 b/qa/2009 new file mode 100755 index 00000000000..f9cb5729809 --- /dev/null +++ b/qa/2009 @@ -0,0 +1,85 @@ +#!/bin/sh +# PCP QA Test No. 2009 +# __pmLogLoadMeta: reject undersized metadata records (rlen < 0). +# +# Crafted archives with 0 < record len < sizeof(__pmLogHdr)+sizeof(trailer) +# must fail cleanly with PM_ERR_LOGREC ("Corrupted record"), not attempt a +# huge malloc via negative rlen converted to size_t. +# +# Copyright (c) 2026 Red Hat. All Rights Reserved. +# + +seq=`basename $0` +echo "QA output created by $seq" + +. ./common.product +. ./common.filter +. ./common.check + +which python3 >/dev/null 2>&1 || _notrun "no python3 binary installed" + +status=1 # failure is the default! +trap "cd $here; rm -rf $tmp; exit \$status" 0 1 2 3 15 +mkdir $tmp + +_filter() +{ + sed -e "s;$tmp;TMP;g" +} + +# Build an archive whose .meta has a valid V2 label followed by a single +# crafted metadata header (len, type) and a little padding. Data/index +# come from archives/ok-foo so volume labels stay consistent. +# +_make_badmeta() +{ + _len="$1" + _type="$2" + _out="$3" + + dd if=archives/ok-foo.meta bs=1 count=132 of=$_out.meta 2>/dev/null + python3 -c " +import struct, sys +# big-endian __pmLogHdr: len + type, plus padding so the header read +# does not sit exactly at EOF +sys.stdout.buffer.write(struct.pack('>II', $_len, $_type) + bytes(32)) +" >>$_out.meta + cp archives/ok-foo.0 $_out.0 + cp archives/ok-foo.index $_out.index +} + +# real QA test starts here + +echo "=== control: unmodified archives/ok-foo opens ===" +if pmdumplog -l archives/ok-foo >/dev/null 2>&1 +then + echo OK +else + echo FAILED +fi + +echo +echo "=== undersized metadata records (expect Corrupted record) ===" +# TYPE_LABEL (7) and TYPE_TEXT (4) are the paths that used to pass a +# negative rlen to malloc(); TYPE_DESC (1) is included for coverage. +# Lengths 1..11 are all below sizeof(__pmLogHdr)+sizeof(int) == 12. +for type in 7 4 1 +do + case $type + in + 1) tname=TYPE_DESC ;; + 4) tname=TYPE_TEXT ;; + 7) tname=TYPE_LABEL ;; + esac + for len in 1 4 8 11 + do + echo + echo "--- len=$len $tname ---" + _make_badmeta $len $type $tmp/bad + pmdumplog -l $tmp/bad 2>&1 | _filter + done +done + +# success, all done +status=0 +exit diff --git a/qa/2009.out b/qa/2009.out new file mode 100644 index 00000000000..0249a2b7bfe --- /dev/null +++ b/qa/2009.out @@ -0,0 +1,41 @@ +QA output created by 2009 +=== control: unmodified archives/ok-foo opens === +OK + +=== undersized metadata records (expect Corrupted record) === + +--- len=1 TYPE_LABEL --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=4 TYPE_LABEL --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=8 TYPE_LABEL --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=11 TYPE_LABEL --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=1 TYPE_TEXT --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=4 TYPE_TEXT --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=8 TYPE_TEXT --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=11 TYPE_TEXT --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=1 TYPE_DESC --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=4 TYPE_DESC --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=8 TYPE_DESC --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=11 TYPE_DESC --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive diff --git a/qa/group b/qa/group index 593e5b18082..3ad96bb46dd 100644 --- a/qa/group +++ b/qa/group @@ -2406,5 +2406,6 @@ suse 2001 pmda.btrfs local 2002 pmda.db2 local 2008 libpcp labels local +2009 libpcp archive local 4751 libpcp threads valgrind local pcp helgrind 9000 other local diff --git a/src/libpcp/src/logmeta.c b/src/libpcp/src/logmeta.c index f8499271754..00d79d0afef 100644 --- a/src/libpcp/src/logmeta.c +++ b/src/libpcp/src/logmeta.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013-2018,2020-2025 Red Hat. + * Copyright (c) 2013-2018,2020-2026 Red Hat. * Copyright (c) 1995-2002 Silicon Graphics, Inc. All Rights Reserved. * * This library is free software; you can redistribute it and/or modify it @@ -818,6 +818,13 @@ __pmLogLoadMeta(__pmArchCtl *acp) nrec[h.type]++; } rlen = h.len - (int)sizeof(__pmLogHdr) - (int)sizeof(int); + if (rlen < 0) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "__pmLogLoadMeta: record len=%d too small (min %d)\n", + h.len, (int)(sizeof(__pmLogHdr) + sizeof(int))); + sts = PM_ERR_LOGREC; + goto end; + } if (h.type == TYPE_DESC) { pmDesc desc; diff --git a/src/libpcp3/src/logmeta.c b/src/libpcp3/src/logmeta.c index 70c687b0ecf..f2accb4ab92 100644 --- a/src/libpcp3/src/logmeta.c +++ b/src/libpcp3/src/logmeta.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013-2018,2020-2025 Red Hat. + * Copyright (c) 2013-2018,2020-2026 Red Hat. * Copyright (c) 1995-2002 Silicon Graphics, Inc. All Rights Reserved. * * This library is free software; you can redistribute it and/or modify it @@ -764,6 +764,13 @@ __pmLogLoadMeta(__pmArchCtl *acp) nrec[h.type]++; } rlen = h.len - (int)sizeof(__pmLogHdr) - (int)sizeof(int); + if (rlen < 0) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "__pmLogLoadMeta: record len=%d too small (min %d)\n", + h.len, (int)(sizeof(__pmLogHdr) + sizeof(int))); + sts = PM_ERR_LOGREC; + goto end; + } if (h.type == TYPE_DESC) { pmDesc desc; From 314a899c46f2282d3a9fdc4053025ef03f0251c3 Mon Sep 17 00:00:00 2001 From: Jan Kurik Date: Fri, 24 Jul 2026 08:40:08 +0200 Subject: [PATCH 2/2] libpcp: bound metadata field reads by remaining record length Strengthen __pmLogLoadMeta() beyond the rlen < 0 guard: reject empty payloads (rlen <= 0), track remaining body bytes for TYPE_DESC (including name length vs MAXPATHLEN), and enforce type-specific minimum sizes for INDOM/LABEL/TEXT before malloc/parse. Extend qa/2009 to cover total record lengths 12..19. Co-authored-by: Cursor --- qa/2009 | 18 ++--- qa/2009.out | 72 +++++++++++++++++++ src/libpcp/src/logmeta.c | 146 ++++++++++++++++++++++++++++++++------ src/libpcp3/src/logmeta.c | 146 ++++++++++++++++++++++++++++++++------ 4 files changed, 330 insertions(+), 52 deletions(-) diff --git a/qa/2009 b/qa/2009 index f9cb5729809..4e5162ff346 100755 --- a/qa/2009 +++ b/qa/2009 @@ -1,10 +1,10 @@ #!/bin/sh # PCP QA Test No. 2009 -# __pmLogLoadMeta: reject undersized metadata records (rlen < 0). +# __pmLogLoadMeta: reject undersized metadata records. # -# Crafted archives with 0 < record len < sizeof(__pmLogHdr)+sizeof(trailer) -# must fail cleanly with PM_ERR_LOGREC ("Corrupted record"), not attempt a -# huge malloc via negative rlen converted to size_t. +# Crafted archives with a record length below the minimum for the declared +# type must fail cleanly with PM_ERR_LOGREC ("Corrupted record"), not +# underflow rlen into a huge allocation or read past the record body. # # Copyright (c) 2026 Red Hat. All Rights Reserved. # @@ -60,9 +60,11 @@ fi echo echo "=== undersized metadata records (expect Corrupted record) ===" -# TYPE_LABEL (7) and TYPE_TEXT (4) are the paths that used to pass a -# negative rlen to malloc(); TYPE_DESC (1) is included for coverage. -# Lengths 1..11 are all below sizeof(__pmLogHdr)+sizeof(int) == 12. +# TYPE_LABEL (7) and TYPE_TEXT (4) used to pass a negative/tiny rlen to +# malloc() or parse past the payload; TYPE_DESC (1) ignored rlen and could +# read past the declared record. Lengths 1..11 fail the rlen <= 0 check +# (body too small for header+trailer). Lengths 12..19 have a non-negative +# but still undersized body for these record types. for type in 7 4 1 do case $type @@ -71,7 +73,7 @@ do 4) tname=TYPE_TEXT ;; 7) tname=TYPE_LABEL ;; esac - for len in 1 4 8 11 + for len in 1 4 8 11 12 13 14 15 16 17 18 19 do echo echo "--- len=$len $tname ---" diff --git a/qa/2009.out b/qa/2009.out index 0249a2b7bfe..26a760321bf 100644 --- a/qa/2009.out +++ b/qa/2009.out @@ -16,6 +16,30 @@ pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive --- len=11 TYPE_LABEL --- pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive +--- len=12 TYPE_LABEL --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=13 TYPE_LABEL --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=14 TYPE_LABEL --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=15 TYPE_LABEL --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=16 TYPE_LABEL --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=17 TYPE_LABEL --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=18 TYPE_LABEL --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=19 TYPE_LABEL --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + --- len=1 TYPE_TEXT --- pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive @@ -28,6 +52,30 @@ pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive --- len=11 TYPE_TEXT --- pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive +--- len=12 TYPE_TEXT --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=13 TYPE_TEXT --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=14 TYPE_TEXT --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=15 TYPE_TEXT --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=16 TYPE_TEXT --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=17 TYPE_TEXT --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=18 TYPE_TEXT --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=19 TYPE_TEXT --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + --- len=1 TYPE_DESC --- pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive @@ -39,3 +87,27 @@ pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive --- len=11 TYPE_DESC --- pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=12 TYPE_DESC --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=13 TYPE_DESC --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=14 TYPE_DESC --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=15 TYPE_DESC --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=16 TYPE_DESC --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=17 TYPE_DESC --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=18 TYPE_DESC --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive + +--- len=19 TYPE_DESC --- +pmdumplog: Cannot open archive "TMP/bad": Corrupted record in a PCP archive diff --git a/src/libpcp/src/logmeta.c b/src/libpcp/src/logmeta.c index 00d79d0afef..6120d11c877 100644 --- a/src/libpcp/src/logmeta.c +++ b/src/libpcp/src/logmeta.c @@ -18,6 +18,7 @@ #include "fault.h" #include "internal.h" #include +#include #include /* bytes for a length field in a header/trailer, or a string length field */ @@ -817,18 +818,33 @@ __pmLogLoadMeta(__pmArchCtl *acp) else nrec[h.type]++; } + /* + * Record length includes the __pmLogHdr and the trailing length + * trailer. Reject non-positive body lengths: rlen < 0 underflows + * into a huge size_t for malloc()/__pmFread(), and rlen == 0 is not + * a valid metadata payload (same class of bug as pmaGetLog() / + * __pmLogRead()). + */ rlen = h.len - (int)sizeof(__pmLogHdr) - (int)sizeof(int); - if (rlen < 0) { + if (rlen <= 0) { if (pmDebugOptions.logmeta) fprintf(stderr, "__pmLogLoadMeta: record len=%d too small (min %d)\n", - h.len, (int)(sizeof(__pmLogHdr) + sizeof(int))); + h.len, (int)(sizeof(__pmLogHdr) + sizeof(int) + 1)); sts = PM_ERR_LOGREC; goto end; } if (h.type == TYPE_DESC) { pmDesc desc; + int left = rlen; numpmid++; + if (left < (int)sizeof(pmDesc)) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "__pmLogLoadMeta: TYPE_DESC rlen=%d too small for pmDesc\n", + rlen); + sts = PM_ERR_LOGREC; + goto end; + } if ((n = (int)__pmFread(&desc, 1, sizeof(pmDesc), f)) != sizeof(pmDesc)) { if (pmDebugOptions.logmeta) { fprintf(stderr, "__pmLogLoadMeta: pmDesc read -> %d: expected: %d\n", @@ -842,6 +858,7 @@ __pmLogLoadMeta(__pmArchCtl *acp) sts = PM_ERR_LOGREC; goto end; } + left -= (int)sizeof(pmDesc); /* swab desc */ desc.type = ntohl(desc.type); @@ -853,28 +870,55 @@ __pmLogLoadMeta(__pmArchCtl *acp) if ((sts = __pmLogAddDesc(acp, &desc)) < 0) goto end; - /* read in the names & store in PMNS tree ... */ - if ((n = (int)__pmFread(&numnames, 1, sizeof(numnames), f)) != - sizeof(numnames)) { - if (pmDebugOptions.logmeta) { - fprintf(stderr, "%s: numnames read -> %d: expected: %d\n", - "__pmLogLoadMeta", n, (int)sizeof(numnames)); + /* + * Zero-name TYPE_DESC records are hdr+desc+trailer only + * (rlen == sizeof(pmDesc)). Otherwise numnames and names + * must fit in the remaining body. + */ + if (left == 0) + numnames = 0; + else { + if (left < (int)sizeof(numnames)) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "__pmLogLoadMeta: TYPE_DESC rlen=%d too small for numnames\n", + rlen); + sts = PM_ERR_LOGREC; + goto end; } - if (__pmFerror(f)) { - __pmClearerr(f); - sts = -oserror(); + if ((n = (int)__pmFread(&numnames, 1, sizeof(numnames), f)) != + sizeof(numnames)) { + if (pmDebugOptions.logmeta) { + fprintf(stderr, "%s: numnames read -> %d: expected: %d\n", + "__pmLogLoadMeta", n, (int)sizeof(numnames)); + } + if (__pmFerror(f)) { + __pmClearerr(f); + sts = -oserror(); + } + else + sts = PM_ERR_LOGREC; + goto end; } - else - sts = PM_ERR_LOGREC; - goto end; - } - else { - /* swab numnames */ + left -= (int)sizeof(numnames); numnames = ntohl(numnames); + if (numnames < 0) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "__pmLogLoadMeta: bad numnames %d\n", + numnames); + sts = PM_ERR_LOGREC; + goto end; + } } for (i = 0; i < numnames; i++) { - if ((n = (int)__pmFread(&len, 1, sizeof(len), f)) != + if (left < (int)sizeof(len)) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "%s: name[%d] length prefix exceeds record\n", + "__pmLogLoadMeta", i); + sts = PM_ERR_LOGREC; + goto end; + } + if ((n = (int)__pmFread(&len, 1, sizeof(len), f)) != sizeof(len)) { if (pmDebugOptions.logmeta) { fprintf(stderr, "%s: len name[%d] read -> %d: expected: %d\n", @@ -888,9 +932,18 @@ __pmLogLoadMeta(__pmArchCtl *acp) sts = PM_ERR_LOGREC; goto end; } - else { - /* swab len */ - len = ntohl(len); + left -= (int)sizeof(len); + len = ntohl(len); + /* + * name[] is MAXPATHLEN; reject oversize or record-crossing + * lengths before the stack copy and NUL terminator write. + */ + if (len < 0 || len >= MAXPATHLEN || len > left) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "%s: name[%d] bad len=%d (left=%d)\n", + "__pmLogLoadMeta", i, len, left); + sts = PM_ERR_LOGREC; + goto end; } if ((n = (int)__pmFread(name, 1, len, f)) != len) { @@ -906,6 +959,7 @@ __pmLogLoadMeta(__pmArchCtl *acp) sts = PM_ERR_LOGREC; goto end; } + left -= len; name[len] = '\0'; /* Add the new PMNS node into this context */ @@ -928,11 +982,29 @@ __pmLogLoadMeta(__pmArchCtl *acp) if (sts < 0) goto end; }/*for*/ + if (left != 0) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "__pmLogLoadMeta: TYPE_DESC %d trailing body bytes\n", + left); + sts = PM_ERR_LOGREC; + goto end; + } } else if (h.type == TYPE_INDOM || h.type == TYPE_INDOM_DELTA || h.type == TYPE_INDOM_V2) { __pmLogInDom lid; __int32_t *buf; - + int minindom; + + /* timestamp + indom + numinst (v3 has an extra timestamp word) */ + minindom = (h.type == TYPE_INDOM_V2) ? + 4 * (int)sizeof(__int32_t) : 5 * (int)sizeof(__int32_t); + if (rlen < minindom) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "__pmLogLoadMeta: TYPE_INDOM* rlen=%d too small (min %d)\n", + rlen, minindom); + sts = PM_ERR_LOGREC; + goto end; + } if ((sts = __pmLogLoadInDom(acp, rlen, h.type, &lid, &buf)) < 0) { goto end; } @@ -971,6 +1043,19 @@ __pmLogLoadMeta(__pmArchCtl *acp) int nsets; pmLabelSet *labelsets; char *tbuf; + int minlabel; + + /* timestamp + type + ident + nsets */ + minlabel = (h.type == TYPE_LABEL_V2) ? + (2 * (int)sizeof(__int32_t) + 3 * (int)sizeof(int)) : + ((int)sizeof(__uint64_t) + (int)sizeof(__int32_t) + 3 * (int)sizeof(int)); + if (rlen < minlabel) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "__pmLogLoadMeta: TYPE_LABEL* rlen=%d too small (min %d)\n", + rlen, minlabel); + sts = PM_ERR_LOGREC; + goto end; + } PM_FAULT_POINT("libpcp/" __FILE__ ":11", PM_FAULT_ALLOC); if ((tbuf = (char *)malloc(rlen)) == NULL) { @@ -1017,6 +1102,15 @@ PM_FAULT_POINT("libpcp/" __FILE__ ":11", PM_FAULT_ALLOC); int type; int ident; int k; + int mintext = (int)sizeof(int) + (int)sizeof(int) + 1; /* type+ident+NUL */ + + if (rlen < mintext) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "__pmLogLoadMeta: TYPE_TEXT rlen=%d too small (min %d)\n", + rlen, mintext); + sts = PM_ERR_LOGREC; + goto end; + } PM_FAULT_POINT("libpcp/" __FILE__ ":16", PM_FAULT_ALLOC); if ((tbuf = (char *)malloc(rlen)) == NULL) { @@ -1062,6 +1156,14 @@ PM_FAULT_POINT("libpcp/" __FILE__ ":16", PM_FAULT_ALLOC); continue; } k += sizeof(ident); + /* help text must be NUL-terminated within the record body */ + if (memchr(&tbuf[k], '\0', rlen - k) == NULL) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "__pmLogLoadMeta: TYPE_TEXT missing NUL\n"); + free(tbuf); + sts = PM_ERR_LOGREC; + goto end; + } sts = addtext(acp, ident, type, (char *)&tbuf[k]); free(tbuf); diff --git a/src/libpcp3/src/logmeta.c b/src/libpcp3/src/logmeta.c index f2accb4ab92..ff8555e5cfe 100644 --- a/src/libpcp3/src/logmeta.c +++ b/src/libpcp3/src/logmeta.c @@ -18,6 +18,7 @@ #include "fault.h" #include "internal.h" #include +#include #include /* bytes for a length field in a header/trailer, or a string length field */ @@ -763,18 +764,33 @@ __pmLogLoadMeta(__pmArchCtl *acp) else nrec[h.type]++; } + /* + * Record length includes the __pmLogHdr and the trailing length + * trailer. Reject non-positive body lengths: rlen < 0 underflows + * into a huge size_t for malloc()/__pmFread(), and rlen == 0 is not + * a valid metadata payload (same class of bug as pmaGetLog() / + * __pmLogRead()). + */ rlen = h.len - (int)sizeof(__pmLogHdr) - (int)sizeof(int); - if (rlen < 0) { + if (rlen <= 0) { if (pmDebugOptions.logmeta) fprintf(stderr, "__pmLogLoadMeta: record len=%d too small (min %d)\n", - h.len, (int)(sizeof(__pmLogHdr) + sizeof(int))); + h.len, (int)(sizeof(__pmLogHdr) + sizeof(int) + 1)); sts = PM_ERR_LOGREC; goto end; } if (h.type == TYPE_DESC) { pmDesc desc; + int left = rlen; numpmid++; + if (left < (int)sizeof(pmDesc)) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "__pmLogLoadMeta: TYPE_DESC rlen=%d too small for pmDesc\n", + rlen); + sts = PM_ERR_LOGREC; + goto end; + } if ((n = (int)__pmFread(&desc, 1, sizeof(pmDesc), f)) != sizeof(pmDesc)) { if (pmDebugOptions.logmeta) { fprintf(stderr, "__pmLogLoadMeta: pmDesc read -> %d: expected: %d\n", @@ -788,6 +804,7 @@ __pmLogLoadMeta(__pmArchCtl *acp) sts = PM_ERR_LOGREC; goto end; } + left -= (int)sizeof(pmDesc); /* swab desc */ desc.type = ntohl(desc.type); @@ -799,28 +816,55 @@ __pmLogLoadMeta(__pmArchCtl *acp) if ((sts = __pmLogAddDesc(acp, &desc)) < 0) goto end; - /* read in the names & store in PMNS tree ... */ - if ((n = (int)__pmFread(&numnames, 1, sizeof(numnames), f)) != - sizeof(numnames)) { - if (pmDebugOptions.logmeta) { - fprintf(stderr, "%s: numnames read -> %d: expected: %d\n", - "__pmLogLoadMeta", n, (int)sizeof(numnames)); + /* + * Zero-name TYPE_DESC records are hdr+desc+trailer only + * (rlen == sizeof(pmDesc)). Otherwise numnames and names + * must fit in the remaining body. + */ + if (left == 0) + numnames = 0; + else { + if (left < (int)sizeof(numnames)) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "__pmLogLoadMeta: TYPE_DESC rlen=%d too small for numnames\n", + rlen); + sts = PM_ERR_LOGREC; + goto end; } - if (__pmFerror(f)) { - __pmClearerr(f); - sts = -oserror(); + if ((n = (int)__pmFread(&numnames, 1, sizeof(numnames), f)) != + sizeof(numnames)) { + if (pmDebugOptions.logmeta) { + fprintf(stderr, "%s: numnames read -> %d: expected: %d\n", + "__pmLogLoadMeta", n, (int)sizeof(numnames)); + } + if (__pmFerror(f)) { + __pmClearerr(f); + sts = -oserror(); + } + else + sts = PM_ERR_LOGREC; + goto end; } - else - sts = PM_ERR_LOGREC; - goto end; - } - else { - /* swab numnames */ + left -= (int)sizeof(numnames); numnames = ntohl(numnames); + if (numnames < 0) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "__pmLogLoadMeta: bad numnames %d\n", + numnames); + sts = PM_ERR_LOGREC; + goto end; + } } for (i = 0; i < numnames; i++) { - if ((n = (int)__pmFread(&len, 1, sizeof(len), f)) != + if (left < (int)sizeof(len)) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "%s: name[%d] length prefix exceeds record\n", + "__pmLogLoadMeta", i); + sts = PM_ERR_LOGREC; + goto end; + } + if ((n = (int)__pmFread(&len, 1, sizeof(len), f)) != sizeof(len)) { if (pmDebugOptions.logmeta) { fprintf(stderr, "%s: len name[%d] read -> %d: expected: %d\n", @@ -834,9 +878,18 @@ __pmLogLoadMeta(__pmArchCtl *acp) sts = PM_ERR_LOGREC; goto end; } - else { - /* swab len */ - len = ntohl(len); + left -= (int)sizeof(len); + len = ntohl(len); + /* + * name[] is MAXPATHLEN; reject oversize or record-crossing + * lengths before the stack copy and NUL terminator write. + */ + if (len < 0 || len >= MAXPATHLEN || len > left) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "%s: name[%d] bad len=%d (left=%d)\n", + "__pmLogLoadMeta", i, len, left); + sts = PM_ERR_LOGREC; + goto end; } if ((n = (int)__pmFread(name, 1, len, f)) != len) { @@ -852,6 +905,7 @@ __pmLogLoadMeta(__pmArchCtl *acp) sts = PM_ERR_LOGREC; goto end; } + left -= len; name[len] = '\0'; /* Add the new PMNS node into this context */ @@ -874,11 +928,29 @@ __pmLogLoadMeta(__pmArchCtl *acp) if (sts < 0) goto end; }/*for*/ + if (left != 0) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "__pmLogLoadMeta: TYPE_DESC %d trailing body bytes\n", + left); + sts = PM_ERR_LOGREC; + goto end; + } } else if (h.type == TYPE_INDOM || h.type == TYPE_INDOM_DELTA || h.type == TYPE_INDOM_V2) { __pmLogInDom lid; __int32_t *buf; - + int minindom; + + /* timestamp + indom + numinst (v3 has an extra timestamp word) */ + minindom = (h.type == TYPE_INDOM_V2) ? + 4 * (int)sizeof(__int32_t) : 5 * (int)sizeof(__int32_t); + if (rlen < minindom) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "__pmLogLoadMeta: TYPE_INDOM* rlen=%d too small (min %d)\n", + rlen, minindom); + sts = PM_ERR_LOGREC; + goto end; + } if ((sts = __pmLogLoadInDom(acp, rlen, h.type, &lid, &buf)) < 0) { goto end; } @@ -917,6 +989,19 @@ __pmLogLoadMeta(__pmArchCtl *acp) int nsets; pmLabelSet *labelsets; char *tbuf; + int minlabel; + + /* timestamp + type + ident + nsets */ + minlabel = (h.type == TYPE_LABEL_V2) ? + (2 * (int)sizeof(__int32_t) + 3 * (int)sizeof(int)) : + ((int)sizeof(__uint64_t) + (int)sizeof(__int32_t) + 3 * (int)sizeof(int)); + if (rlen < minlabel) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "__pmLogLoadMeta: TYPE_LABEL* rlen=%d too small (min %d)\n", + rlen, minlabel); + sts = PM_ERR_LOGREC; + goto end; + } PM_FAULT_POINT("libpcp/" __FILE__ ":11", PM_FAULT_ALLOC); if ((tbuf = (char *)malloc(rlen)) == NULL) { @@ -951,6 +1036,15 @@ PM_FAULT_POINT("libpcp/" __FILE__ ":11", PM_FAULT_ALLOC); int type; int ident; int k; + int mintext = (int)sizeof(int) + (int)sizeof(int) + 1; /* type+ident+NUL */ + + if (rlen < mintext) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "__pmLogLoadMeta: TYPE_TEXT rlen=%d too small (min %d)\n", + rlen, mintext); + sts = PM_ERR_LOGREC; + goto end; + } PM_FAULT_POINT("libpcp/" __FILE__ ":16", PM_FAULT_ALLOC); if ((tbuf = (char *)malloc(rlen)) == NULL) { @@ -996,6 +1090,14 @@ PM_FAULT_POINT("libpcp/" __FILE__ ":16", PM_FAULT_ALLOC); continue; } k += sizeof(ident); + /* help text must be NUL-terminated within the record body */ + if (memchr(&tbuf[k], '\0', rlen - k) == NULL) { + if (pmDebugOptions.logmeta) + fprintf(stderr, "__pmLogLoadMeta: TYPE_TEXT missing NUL\n"); + free(tbuf); + sts = PM_ERR_LOGREC; + goto end; + } sts = addtext(acp, ident, type, (char *)&tbuf[k]); free(tbuf);