Skip to content

libpcp: fix heap buffer over-read in __pmLogLoadLabelSet #2667

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lilu5458 wants to merge 1 commit into
base: main
Choose a base branch
from
+30 −0
Open

libpcp: fix heap buffer over-read in __pmLogLoadLabelSet #2667

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/libpcp/src/e_labels.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,11 @@ __pmLogLoadLabelSet(char *tbuf, int rlen, int rtype, __pmTimestamp *stamp,
}

for (i = 0; i < nsets; i++) {
if (k + 2*sizeof(__int32_t) > rlen) {
sts = PM_ERR_LOGREC;
free(labelsets);
return sts;
}
Comment on lines +245 to +249

@coderabbitai coderabbitai Bot Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Use recursive labelset cleanup on every new error exit.

  • src/libpcp/src/e_labels.c#L245-L249: free all previously decoded sets, not only labelsets.
  • src/libpcp/src/e_labels.c#L268-L271: free nested allocations from earlier sets before returning.
  • src/libpcp/src/e_labels.c#L284-L288: include the current JSON allocation and all earlier sets.
  • src/libpcp3/src/e_labels.c#L250-L254: apply the same cleanup before the current set is allocated.
  • src/libpcp3/src/e_labels.c#L273-L277: free nested allocations from earlier sets.
  • src/libpcp3/src/e_labels.c#L289-L293: include the current JSON allocation and all earlier sets.

The callers terminate on negative status, so they cannot repair these leaks.

📍 Affects 2 files
  • src/libpcp/src/e_labels.c#L245-L249 (this comment)
  • src/libpcp3/src/e_labels.c#L250-L254
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/libpcp/src/e_labels.c` around lines 245 - 249, The error exits in the
label decoding logic leak previously decoded label sets and nested allocations.
Update the cleanup paths in src/libpcp/src/e_labels.c at lines 245-249, 268-271,
and 284-288, and in src/libpcp3/src/e_labels.c at lines 250-254, 273-277, and
289-293, to use the recursive labelset cleanup routine; include the current JSON
allocation where it exists and all earlier sets before returning the negative
status.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate files =="
git ls-files | rg '(^|/)e_labels\.c$|logmeta\.c$' || true

echo
echo "== src/libpcp/src/e_labels.c outline/size =="
wc -l src/libpcp/src/e_labels.c || true
ast-grep outline src/libpcp/src/e_labels.c || true

echo
echo "== src/libpcp/src/e_labels.c lines 200-290 =="
sed -n '200,290p' src/libpcp/src/e_labels.c

echo
echo "== src/libpcp3/src/e_labels.c lines 200-305 =="
sed -n '200,305p' src/libpcp3/src/e_labels.c

echo
echo "== search for decoder caller context =="
rg -n "e_labels|labelsets|_logmeta|logmeta" src/libpcp src/libpcp3 -g '*.c' -g '*.h' || true

Repository: performancecopilot/pcp

Length of output: 22809

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== e_labels.c relevant call sites =="
sed -n '150,178p' src/libpcp/src/e_labels.c
sed -n '180,198p' src/libpcp/src/e_labels.c
sed -n '180,198p' src/libpcp3/src/e_labels.c

echo
echo "== logmeta.c call sites around __pmLogLoadLabelSet =="
sed -n '950,1005p' src/libpcp/src/logmeta.c
sed -n '910,945p' src/libpcp3/src/logmeta.c

echo
echo "== search for length before addlabel / __pmLogLoadLabelSet =="
rg -n "addlabel|__pmLogLoadLabelSet|rlen" src/libpcp/src/logmeta.c src/libpcp3/src/logmeta.c src/libpcp/src/e_labels.c src/libpcp3/src/e_labels.c

echo
echo "== read-only header-size invariant probe =="
python3 - <<'PY'
from pathlib import Path
import re

def read_file(path):
    return path.read_text()

def extract_decoder(path):
    text=read_file(path)
    m=re.search(r'int\s+__pmLogLoadLabelSet\([^;]*\{\s*(?P<body>.*?)\n(?=\nint\s|$)', text, re.S)
    if not m:
        # broader until first free(labelsets); and return sts after JSON allocation
        lines=text.splitlines()
        start=lines.index(next(l for l in lines if '__pmLogLoadLabelSet' in l))
        body=[]
        brace=0
        for l in lines[start:]:
            body.append(l)
            brace += l.count('{') - l.count('}')
            if brace == 0 or ('free(labelsets)' in l and 'return sts' in l):
                break
        return '\n'.join(body[:140])
    return m.group('body')

for path in [Path('src/libpcp/src/e_labels.c'), Path('src/libpcp3/src/e_labels.c')]:
    body=extract_decoder(path)
    print(f'-- {path} --')
    for line in body.splitlines()[:32]:
        if kcheck in body:
            print(line)
    print('checks:', {
        'early_header_check_before_type_id_read': 'k + 6*sizeof(__int32_t) > rlen' in body or 'k + 2*sizeof(__int32_t) + 3*sizeof(unsigned int)' in body,
        'first_set_guard_present': 'if (k + 2*sizeof(__int32_t) > rlen)' in body,
        'first_set_guard_k': body.find('if (k + 2*sizeof(__int32_t) > rlen)'),
        'first_labelset_cast': body.find('inst = *((unsigned int*)&tbuf[k])'),
    })
    print(body[0:1200])
PY

Repository: performancecopilot/pcp

Length of output: 10126

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== src/libpcp/src/e_labels.c lines 30-210 =="
sed -n '30,210p' src/libpcp/src/e_labels.c

echo
echo "== src/libpcp3/src/e_labels.c lines 30-220 =="
sed -n '30,220p' src/libpcp3/src/e_labels.c

echo
echo "== logmeta label record length parsing around rlen =="
sed -n '740,830p' src/libpcp/src/logmeta.c
sed -n '760,830p' src/libpcp3/src/logmeta.c

Repository: performancecopilot/pcp

Length of output: 14352

Guard rlen before decoding the label record header.

Both __pmLogLoadLabelSet implementations read TYPE_LABEL_V2 timestamps, type, identifier, and nsets before checking k + 2*sizeof(__int32_t) > rlen and allocate/clean up labelsets only after nsets validation. Add a minimum-header-length check at the start of the decoder in src/libpcp/src/e_labels.c and src/libpcp3/src/e_labels.c to reject truncated records before any header field reads.

📍 Affects 2 files
  • src/libpcp/src/e_labels.c#L245-L249 (this comment)
  • src/libpcp3/src/e_labels.c#L250-L254
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/libpcp/src/e_labels.c` around lines 245 - 249, Add a
minimum-header-length guard at the start of both __pmLogLoadLabelSet
implementations, before decoding any TYPE_LABEL_V2 timestamp, type, identifier,
or nsets fields; update src/libpcp/src/e_labels.c lines 245-249 and
src/libpcp3/src/e_labels.c lines 250-254 consistently to reject truncated
records before allocation or field reads.

inst = *((unsigned int*)&tbuf[k]);
inst = ntohl(inst);
k += sizeof(inst);
Expand All @@ -260,6 +265,11 @@ __pmLogLoadLabelSet(char *tbuf, int rlen, int rtype, __pmTimestamp *stamp,
return sts;
}

if (k + jsonlen > rlen) {
free(labelsets);
return PM_ERR_LOGREC;
}
Comment on lines +268 to +271

@coderabbitai coderabbitai Bot Jul 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

Make both JSON bounds checks overflow-safe.

  • src/libpcp/src/e_labels.c#L268-L271: replace k + jsonlen > rlen with a subtraction- or size_t-based check.
  • src/libpcp3/src/e_labels.c#L273-L277: apply the identical overflow-safe comparison.

As per path instructions, C/C++ code must check arithmetic on untrusted sizes.

📍 Affects 2 files
  • src/libpcp/src/e_labels.c#L268-L271 (this comment)
  • src/libpcp3/src/e_labels.c#L273-L277
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/libpcp/src/e_labels.c` around lines 268 - 271, Make the JSON bounds
checks in src/libpcp/src/e_labels.c lines 268-271 and src/libpcp3/src/e_labels.c
lines 273-277 overflow-safe by replacing the k + jsonlen > rlen arithmetic with
a subtraction-based or size_t-based comparison; apply the same validation logic
at both sites before processing untrusted sizes.

Source: Path instructions


if ((labelsets[i].json = (char *)malloc(jsonlen+1)) == NULL) {
sts = -oserror();
free(labelsets);
Expand All @@ -271,6 +281,11 @@ __pmLogLoadLabelSet(char *tbuf, int rlen, int rtype, __pmTimestamp *stamp,
k += jsonlen;

/* label nlabels */
if (k + sizeof(__int32_t) > rlen) {
free(labelsets[i].json);
free(labelsets);
return PM_ERR_LOGREC;
}
nlabels = ntohl(*((unsigned int *)&tbuf[k]));
k += sizeof(nlabels);
labelsets[i].nlabels = nlabels;
Expand Down
15 changes: 15 additions & 0 deletions src/libpcp3/src/e_labels.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,11 @@ __pmLogLoadLabelSet(char *tbuf, int rlen, int rtype, __pmTimestamp *stamp,
}

for (i = 0; i < nsets; i++) {
if (k + 2*sizeof(__int32_t) > rlen) {
sts = PM_ERR_LOGREC;
free(labelsets);
return sts;
}
inst = *((unsigned int*)&tbuf[k]);
inst = ntohl(inst);
k += sizeof(inst);
Expand All @@ -265,6 +270,11 @@ __pmLogLoadLabelSet(char *tbuf, int rlen, int rtype, __pmTimestamp *stamp,
return sts;
}

if (k + jsonlen > rlen) {
free(labelsets);
return PM_ERR_LOGREC;
}

if ((labelsets[i].json = (char *)malloc(jsonlen+1)) == NULL) {
sts = -oserror();
free(labelsets);
Expand All @@ -276,6 +286,11 @@ __pmLogLoadLabelSet(char *tbuf, int rlen, int rtype, __pmTimestamp *stamp,
k += jsonlen;

/* label nlabels */
if (k + sizeof(__int32_t) > rlen) {
free(labelsets[i].json);
free(labelsets);
return PM_ERR_LOGREC;
}
nlabels = ntohl(*((unsigned int *)&tbuf[k]));
k += sizeof(nlabels);
labelsets[i].nlabels = nlabels;
Expand Down