Skip to content

Harden raw KV deserialization#8102

Open
achamayou wants to merge 2 commits into
mainfrom
achamayou-harden-kv-deserialization
Open

Harden raw KV deserialization#8102
achamayou wants to merge 2 commits into
mainfrom
achamayou-harden-kv-deserialization

Conversation

@achamayou

@achamayou achamayou commented Jul 26, 2026

Copy link
Copy Markdown
Member

Summary

  • Validate fixed-size and size-prefixed reads before advancing the raw KV reader cursor.
  • Reject oversized public domains, truncated arrays, invalid vector byte lengths, and null buffers with non-zero sizes.
  • Add focused KV serialization regressions for malformed and truncated input.

Context

Extracted from #8092 so this generic parser hardening can be reviewed and landed independently. #8092 should be rebased onto this change: its recovery path parses untrusted snapshot and ledger bytes through RawKvStoreDeserialiser, so it relies on these checks as a prerequisite.

Validation

  • C++ formatting
  • ASCII and public-header checks
  • Focused malformed-input coverage added to kv_test

@achamayou
Harden raw KV deserialization
837f17c 
Validate fixed-width, size-prefixed, vector, and public-domain lengths before advancing pointers, allocating, or copying. Add regressions for truncated and invalid input.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: bc1a467e-7d08-4580-886e-64af3a0e514c
Copilot AI review requested due to automatic review settings July 26, 2026 18:54
@achamayou
achamayou requested a review from a team as a code owner July 26, 2026 18:54
@achamayou achamayou mentioned this pull request Jul 26, 2026
Draft
Copilot AI reviewed Jul 26, 2026
View reviewed changes

Copilot AI left a comment

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.

Pull request overview

This PR hardens CCF’s raw KV deserialisation path (used by RawKvStoreDeserialiser) by adding strict bounds checks before cursor advancement and by rejecting malformed size-prefixed/vector/array reads and invalid public-domain slices, with regression tests covering truncated and inconsistent inputs.

Changes:

  • Add RawReader invariants and pre-read bounds checks (fixed-size, size-prefixed, arrays/vectors), plus invalid-argument rejection for null buffers with non-zero sizes.
  • Validate public_domain_length against remaining entry size before initialising the public-domain reader.
  • Add focused kv_test cases for truncated fixed-width reads, inconsistent size prefixes, non-divisible vector payloads, and malformed public-domain payloads (including truncated claims digests).

Custom instructions used:

  • .github/copilot-instructions.md
  • .github/instructions/reviewing.instructions.md

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/kv/test/kv_serialisation.cpp Adds regression tests for truncated/malformed raw reads and invalid public-domain entry layouts.
src/kv/raw_serialise.h Hardens RawReader with cursor/size invariants, explicit bounds checks, and safer vector/array reads.
src/kv/generic_serialise_wrapper.h Rejects public-domain lengths that exceed remaining entry bytes before advancing/initialising the reader.

@github-actions

github-actions Bot commented Jul 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Description

Comparing 3 available runs from this branch (#8102) against the trend of the last 30 main runs.

Each chart plots every benchmark as an axis, with values normalized so 100 is the EWMA baseline of recent main runs, using a 7-run half-life. The 3 orange branch lines run from the oldest (faintest) to the latest (darkest and thickest); the darker blue band is the main baseline +/- 1 std dev and the lighter blue band around it is +/- 2 std dev.

Axis labels show the latest branch value and its difference from the main EWMA baseline, where 0% is on the baseline. They are coloured green where the latest run improves on the baseline, red where it regresses, and grey where the difference is within one std dev of the baseline (within noise). Higher is better for throughput and rate, lower for latency and memory.

Throughput (tx/s)

---
config:
  radar:
    width: 620
    height: 620
    marginTop: 90
    marginRight: 220
    marginBottom: 60
    marginLeft: 220
    axisLabelFactor: 1.12
    curveTension: 0.08
  theme: base
  themeCSS: |
    .radarCurve-0{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-1{fill:color-mix(in srgb, #62B5E5 40%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-2{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-3{fill:var(--color-canvas-default,var(--bgColor-default,#fff))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarAxisLabel,.radarTitle{fill:var(--color-fg-default,var(--fgColor-default,#111827))!important;color:var(--color-fg-default,var(--fgColor-default,#111827))!important}
    .radarCurve-4{stroke-width:1.5px!important;stroke-opacity:0.40!important}
    .radarCurve-5{stroke-width:1.5px!important;stroke-opacity:0.50!important}
    .radarCurve-6{stroke-width:1.75px!important;stroke-opacity:1.00!important}
    .radarAxisLabel:nth-of-type(1){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(2){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(3){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(4){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(5){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(6){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(7){fill:#808A94!important}
  themeVariables:
    cScale0: "#62B5E5"
    cScale1: "#62B5E5"
    cScale2: "#62B5E5"
    cScale3: "#62B5E5"
    cScale4: "#F97316"
    cScale5: "#F97316"
    cScale6: "#F97316"
    radar:
      axisColor: "#9CA3AF"
      graticuleColor: "#E5E7EB"
      graticuleOpacity: 0
      axisStrokeWidth: 1
      curveOpacity: 0
---
radar-beta
  axis b0["Basic: 65,684 tx/s ▬ +1%"]
  axis b1["Basic Blocking: 1,021 tx/s ▬ +2%"]
  axis b2["Basic JS: 4,726 tx/s ▬ +2%"]
  axis b3["Basic Multi-Threaded: 87,518 tx/s ▬ +4%"]
  axis b4["Historical Queries: 211,592 tx/s ▬ +6%"]
  axis b5["Logging: 65,183 tx/s ▬ +7%"]
  axis b6["Logging JWT: 10,241 tx/s ▬ +4%"]
  curve stddev2_high["main EWMA + 2 std dev"]{116.88, 106.01, 116.04, 116.03, 125.20, 117.42, 117.69}
  curve stddev1_high["main EWMA + 1 std dev"]{108.44, 103.01, 108.02, 108.01, 112.60, 108.71, 108.84}
  curve stddev1_low["main EWMA - 1 std dev"]{91.56, 96.99, 91.98, 91.99, 87.40, 91.29, 91.16}
  curve stddev2_low["main EWMA - 2 std dev"]{83.12, 93.99, 83.96, 83.97, 74.80, 82.58, 82.31}
  curve branch_0["#8102 (2 runs earlier)"]{104.45, 101.81, 101.15, 104.33, 108.76, 103.96, 104.70}
  curve branch_1["#8102 (1 run earlier)"]{103.94, 101.70, 102.69, 103.52, 104.58, 103.95, 98.05}
  curve branch_2["#8102"]{100.89, 101.91, 102.02, 104.39, 106.40, 107.49, 103.94}
  graticule polygon
  max 143
  min 57
  ticks 0
  showLegend false
Loading

Latency (ms)

---
config:
  radar:
    width: 620
    height: 620
    marginTop: 90
    marginRight: 220
    marginBottom: 60
    marginLeft: 220
    axisLabelFactor: 1.12
    curveTension: 0.08
  theme: base
  themeCSS: |
    .radarCurve-0{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-1{fill:color-mix(in srgb, #62B5E5 40%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-2{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-3{fill:var(--color-canvas-default,var(--bgColor-default,#fff))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarAxisLabel,.radarTitle{fill:var(--color-fg-default,var(--fgColor-default,#111827))!important;color:var(--color-fg-default,var(--fgColor-default,#111827))!important}
    .radarCurve-4{stroke-width:1.5px!important;stroke-opacity:0.40!important}
    .radarCurve-5{stroke-width:1.5px!important;stroke-opacity:0.50!important}
    .radarCurve-6{stroke-width:1.75px!important;stroke-opacity:1.00!important}
    .radarAxisLabel:nth-of-type(1){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(2){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(3){fill:#808A94!important}
  themeVariables:
    cScale0: "#62B5E5"
    cScale1: "#62B5E5"
    cScale2: "#62B5E5"
    cScale3: "#62B5E5"
    cScale4: "#F97316"
    cScale5: "#F97316"
    cScale6: "#F97316"
    radar:
      axisColor: "#9CA3AF"
      graticuleColor: "#E5E7EB"
      graticuleOpacity: 0
      axisStrokeWidth: 1
      curveOpacity: 0
---
radar-beta
  axis b0["Commit Latency 16ms: 5.31 ms ▬ -1%"]
  axis b1["Commit Latency 1ms: 1.78 ms ▬ -1%"]
  axis b2["Commit Latency 256ms: 206 ms ▬ -1%"]
  curve stddev2_high["main EWMA + 2 std dev"]{139.15, 107.78, 101.45}
  curve stddev1_high["main EWMA + 1 std dev"]{119.58, 103.89, 100.72}
  curve stddev1_low["main EWMA - 1 std dev"]{80.42, 96.11, 99.28}
  curve stddev2_low["main EWMA - 2 std dev"]{60.85, 92.22, 98.55}
  curve branch_0["#8102 (2 runs earlier)"]{92.44, 100.55, 99.67}
  curve branch_1["#8102 (1 run earlier)"]{80.61, 101.26, 99.94}
  curve branch_2["#8102"]{98.53, 98.57, 99.40}
  graticule polygon
  max 167
  min 33
  ticks 0
  showLegend false
Loading

Memory (bytes)

---
config:
  radar:
    width: 620
    height: 620
    marginTop: 90
    marginRight: 220
    marginBottom: 60
    marginLeft: 220
    axisLabelFactor: 1.12
    curveTension: 0.08
  theme: base
  themeCSS: |
    .radarCurve-0{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-1{fill:color-mix(in srgb, #62B5E5 40%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-2{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-3{fill:var(--color-canvas-default,var(--bgColor-default,#fff))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarAxisLabel,.radarTitle{fill:var(--color-fg-default,var(--fgColor-default,#111827))!important;color:var(--color-fg-default,var(--fgColor-default,#111827))!important}
    .radarCurve-4{stroke-width:1.5px!important;stroke-opacity:0.40!important}
    .radarCurve-5{stroke-width:1.5px!important;stroke-opacity:0.50!important}
    .radarCurve-6{stroke-width:1.75px!important;stroke-opacity:1.00!important}
    .radarAxisLabel:nth-of-type(1){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(2){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(3){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(4){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(5){fill:#E5484D!important}
    .radarAxisLabel:nth-of-type(6){fill:#808A94!important}
  themeVariables:
    cScale0: "#62B5E5"
    cScale1: "#62B5E5"
    cScale2: "#62B5E5"
    cScale3: "#62B5E5"
    cScale4: "#F97316"
    cScale5: "#F97316"
    cScale6: "#F97316"
    radar:
      axisColor: "#9CA3AF"
      graticuleColor: "#E5E7EB"
      graticuleOpacity: 0
      axisStrokeWidth: 1
      curveOpacity: 0
---
radar-beta
  axis b0["Basic: 89.5 MiB ▬ +2%"]
  axis b1["Basic Blocking: 71.3 MiB ▬ 0%"]
  axis b2["Basic JS: 69.8 MiB ▬ -1%"]
  axis b3["Basic Multi-Threaded: 89.5 MiB ▬ 0%"]
  axis b4["Logging: 76.6 MiB ▲ 1%"]
  axis b5["Logging JWT: 68.7 MiB ▬ 0%"]
  curve stddev2_high["main EWMA + 2 std dev"]{106.07, 100.96, 104.08, 102.31, 102.26, 101.47}
  curve stddev1_high["main EWMA + 1 std dev"]{103.03, 100.48, 102.04, 101.15, 101.13, 100.73}
  curve stddev1_low["main EWMA - 1 std dev"]{96.97, 99.52, 97.96, 98.85, 98.87, 99.27}
  curve stddev2_low["main EWMA - 2 std dev"]{93.93, 99.04, 95.92, 97.69, 97.74, 98.53}
  curve branch_0["#8102 (2 runs earlier)"]{94.14, 99.75, 101.70, 98.74, 100.94, 100.59}
  curve branch_1["#8102 (1 run earlier)"]{102.61, 101.14, 98.03, 99.03, 99.97, 101.15}
  curve branch_2["#8102"]{102.22, 99.92, 98.58, 100.30, 101.16, 100.38}
  graticule polygon
  max 111
  min 89
  ticks 0
  showLegend false
Loading

Rate (ops/s)

---
config:
  radar:
    width: 620
    height: 620
    marginTop: 90
    marginRight: 220
    marginBottom: 60
    marginLeft: 220
    axisLabelFactor: 1.12
    curveTension: 0.08
  theme: base
  themeCSS: |
    .radarCurve-0{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-1{fill:color-mix(in srgb, #62B5E5 40%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-2{fill:color-mix(in srgb, #62B5E5 13%, var(--color-canvas-default,var(--bgColor-default,#fff)))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarCurve-3{fill:var(--color-canvas-default,var(--bgColor-default,#fff))!important;fill-opacity:1!important;stroke:none!important;stroke-width:0!important}
    .radarAxisLabel,.radarTitle{fill:var(--color-fg-default,var(--fgColor-default,#111827))!important;color:var(--color-fg-default,var(--fgColor-default,#111827))!important}
    .radarCurve-4{stroke-width:1.5px!important;stroke-opacity:0.40!important}
    .radarCurve-5{stroke-width:1.5px!important;stroke-opacity:0.50!important}
    .radarCurve-6{stroke-width:1.75px!important;stroke-opacity:1.00!important}
    .radarAxisLabel:nth-of-type(1){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(2){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(3){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(4){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(5){fill:#808A94!important}
    .radarAxisLabel:nth-of-type(6){fill:#808A94!important}
  themeVariables:
    cScale0: "#62B5E5"
    cScale1: "#62B5E5"
    cScale2: "#62B5E5"
    cScale3: "#62B5E5"
    cScale4: "#F97316"
    cScale5: "#F97316"
    cScale6: "#F97316"
    radar:
      axisColor: "#9CA3AF"
      graticuleColor: "#E5E7EB"
      graticuleOpacity: 0
      axisStrokeWidth: 1
      curveOpacity: 0
---
radar-beta
  axis b0["CHAMP get: 38,372,899 ops/s ▬ +3%"]
  axis b1["CHAMP put: 5,417,130 ops/s ▬ 0%"]
  axis b2["KV deserialisation: 1,628,930 ops/s ▬ +3%"]
  axis b3["KV serialisation: 1,434,720 ops/s ▬ 0%"]
  axis b4["KV snapshot deserialis...: 4,263 ops/s ▬ +4%"]
  axis b5["KV snapshot serialisation: 4,695 ops/s ▬ +5%"]
  curve stddev2_high["main EWMA + 2 std dev"]{115.06, 115.24, 115.08, 114.62, 114.84, 118.54}
  curve stddev1_high["main EWMA + 1 std dev"]{107.53, 107.62, 107.54, 107.31, 107.42, 109.27}
  curve stddev1_low["main EWMA - 1 std dev"]{92.47, 92.38, 92.46, 92.69, 92.58, 90.73}
  curve stddev2_low["main EWMA - 2 std dev"]{84.94, 84.76, 84.92, 85.38, 85.16, 81.46}
  curve branch_0["#8102 (2 runs earlier)"]{101.49, 102.97, 92.38, 101.55, 103.68, 105.47}
  curve branch_1["#8102 (1 run earlier)"]{100.61, 102.89, 90.38, 103.21, 102.76, 100.03}
  curve branch_2["#8102"]{102.67, 100.27, 102.77, 99.80, 104.19, 105.35}
  graticule polygon
  max 132
  min 68
  ticks 0
  showLegend false
Loading

@achamayou
Reduce raw KV reader bounds-check overhead
db6685c 
Keep a single bounds check before each read or cursor advance, while removing redundant invariant, null, and checked-copy work from the deserialization hot path. Preserve cursor assertions in debug builds.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: bc1a467e-7d08-4580-886e-64af3a0e514c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

bench-ab READY TO MERGE

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@achamayou