Skip to content

Conversation

odaysec
Copy link

@odaysec odaysec commented Sep 20, 2025

Summary of Description

This request introduces several security and robustness improvements across multiple components of the Pyroscope codebase. The changes focus on strengthening input validation, preventing integer overflows, and ensuring safer error handling.

  1. Safe Parsing of Sample Rate (pkg/ingester/pyroscope/ingest_handler.go)

    • Replaced strconv.Atoi with strconv.ParseUint(sr, 10, 32) to ensure that only values safely representable as uint32 are accepted.
    • If parsing fails, a default value is used as before.
    • Prevents unsafe conversions and potential overflows.
  2. Bounds Check for ByteSize (pkg/og/util/bytesize/bytesize.go)

    • Added a check to ensure that parsed values do not exceed math.MaxInt64 before converting to ByteSize (int64).
    • Returns a parse error if the value is out of range.
    • Imported math where required.
  3. Validation for Shards (pkg/querier/replication.go)

    • Introduced validation to ensure that shards is greater than 0 and does not exceed math.MaxInt.
    • Added checks to confirm that shardIdx is within bounds before being used as an index.
    • Prevents invalid slice operations and potential panics.
  4. Safe Handling of Split Count (pkg/storegateway/gateway_blocks_http.go)

    • Replaced strconv.Atoi with strconv.ParseInt(sc, 10, 32) and added validation.
    • Ensured that splitCount is within [0, math.MaxUint32]; otherwise, reset to a safe default (0).
    • Prevents unsafe casting and integer overflows.
  5. Validation of Log Entry Buffer Size (pkg/metastore/fsm/log_entry.go)

    • Introduced a maximum allowed size constant (64 MB) for len(raw).
    • If len(raw) exceeds this threshold, return an error instead of allocating a large buffer.
    • Ensures safe and bounded memory allocation.

Rationale

These changes improve the overall security and stability of Pyroscope by:

  • Preventing unsafe integer conversions.
  • Avoiding integer overflows and out-of-bounds slice usage.
  • Enforcing safer defaults for untrusted input.
  • Reducing the risk of excessive memory allocation.

All modifications are scoped to the mentioned files and functions. No external dependencies were added, and the fixes are fully compatible with existing functionality.

Go Language Integer overflow
Making slices, maps and channels

@odaysec odaysec requested review from aleks-p, alsoba13, marcsanmi and a team as code owners September 20, 2025 13:11
@CLAassistant
Copy link

CLAassistant commented Sep 20, 2025

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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