Skip to content

fix(telemetry): bound processing of prefixed metadata headers - #631

Open
sfc-gh-ikryvanos wants to merge 1 commit into
mainfrom
fix/telemetry-bound-metadata-headers
Open

fix(telemetry): bound processing of prefixed metadata headers#631
sfc-gh-ikryvanos wants to merge 1 commit into
mainfrom
fix/telemetry-bound-metadata-headers

Conversation

@sfc-gh-ikryvanos

Copy link
Copy Markdown
Collaborator

logMetadata attached each prefixed metadata value with its own WithValues call (O(N^2) slice copies) and both it and passAlongMetadata processed an unbounded number of caller-supplied headers before authorization. A flood of headers could therefore exhaust CPU/memory and spam the audit log.

Collect prefixed pairs once, cap the pair count and per-value length, and attach/forward them in a single call. Adds a unit test for the bounds.

logMetadata attached each prefixed metadata value with its own WithValues
call (O(N^2) slice copies) and both it and passAlongMetadata processed an
unbounded number of caller-supplied headers before authorization. A flood
of headers could therefore exhaust CPU/memory and spam the audit log.

Collect prefixed pairs once, cap the pair count and per-value length, and
attach/forward them in a single call. Adds a unit test for the bounds.

Co-authored-by: Cursor <cursoragent@cursor.com>
Comment thread telemetry/telemetry.go
Comment on lines +58 to +70
for k, v := range md {
if !strings.HasPrefix(k, sansshellMetadata) {
continue
}
for _, val := range v {
if len(pairs)/2 >= maxSansshellMetadataPairs {
return pairs
}
if len(val) > maxSansshellMetadataValueLen {
val = val[:maxSansshellMetadataValueLen]
}
pairs = append(pairs, k, val)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 MEDIUM · Unsanitized caller-controlled gRPC metadata logged verbatim, enabling audit-log injection [NEW] · CWE-117

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants