fix(consensus/reactor): reject oversized proposals by aljo242 · Pull Request #5324 · cometbft/cometbft

aljo242 · 2025-09-02T18:04:46Z

Updates the consensus reactor to validate that a received proposal will not contain more parts than the amount of chunks that it would take to build a block whos size is equal to ConsensusParams.Block.MaxBytes.

Original PR is here #5309, but reopened since the contributor stopped replying.

PR checklist

Tests written/updated
Changelog entry added in .changelog (we use unclog to manage our changelog)
Updated relevant documentation (docs/ or spec/) and code comments

Co-authored-by: Abdul Malek <me@almk.dev>

…al is not accepted by a peer

consensus/reactor.go

this repo is blowing up my notifications, removing myself from codeowners for now #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments

```markdown ## Summary This PR addresses minor typographical errors found across multiple files in the codebase, improving code clarity and documentation accuracy. ## Changes Made ### Documentation Files - **`docs/references/architecture/tendermint-core/adr-023-ABCI-propose-tx.md`** - Fixed "Additinoally" → "Additionally" - **`docs/references/architecture/tendermint-core/adr-030-consensus-refactor.md`** - Fixed "MeesageUnknown" → "MessageUnknown" - **`docs/references/config/config.toml.md`** - Fixed "receveing" → "receiving" - **`docs/references/rfc/tendermint-core/rfc-002-ipc-ecosystem.md`** - Fixed "difficultly" → "difficulty" - **`docs/references/rfc/tendermint-core/rfc-008-do-not-panic.md`** - Fixed "espically" → "especially" - **`docs/references/rfc/tendermint-core/rfc-017-abci++-vote-extension-propag.md`** - Fixed "snapshop" → "snapshot" - **`docs/references/storage/README.md`** - Fixed "procssed" → "processed" - **`docs/app-dev/indexing-transactions.md`** - Fixed "Recepient" → "Recipient" ### Code Files - **`crypto/secp256k1/secp256k1.go`** - Fixed "marshalls" → "marshals" in comment - **`types/vote_test.go`** - Fixed "remaning" → "remaining" in comment ``` --------- Co-authored-by: Alex | Interchain Labs <alex@cosmoslabs.io>

types/proposal.go

--- Updates the consensus reactor to validate that a received proposal will not contain more parts than the amount of chunks that it would take to build a block whos size is equal to `ConsensusParams.Block.MaxBytes`. Original PR is here #5309, but reopened since the contributor stopped replying. #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments --------- Co-authored-by: arsushi <richie@asymmetric.re> Co-authored-by: Abdul Malek <me@almk.dev> Co-authored-by: Matt Acciai <matt@skip.money> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Tyler <48813565+technicallyty@users.noreply.github.com> Co-authored-by: maradini77 <140460067+maradini77@users.noreply.github.com> (cherry picked from commit 9ff47d0)

…5407) --- Updates the consensus reactor to validate that a received proposal will not contain more parts than the amount of chunks that it would take to build a block whos size is equal to `ConsensusParams.Block.MaxBytes`. Original PR is here #5309, but reopened since the contributor stopped replying. #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments <hr>This is an automatic backport of pull request #5324 done by [Mergify](https://mergify.com). Co-authored-by: Alex | Interchain Labs <alex@cosmoslabs.io> Co-authored-by: arsushi <richie@asymmetric.re> Co-authored-by: Abdul Malek <me@almk.dev> Co-authored-by: Matt Acciai <matt@skip.money> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Tyler <48813565+technicallyty@users.noreply.github.com> Co-authored-by: maradini77 <140460067+maradini77@users.noreply.github.com>

…#5324) (cometbft#5407) --- Updates the consensus reactor to validate that a received proposal will not contain more parts than the amount of chunks that it would take to build a block whos size is equal to `ConsensusParams.Block.MaxBytes`. Original PR is here cometbft#5309, but reopened since the contributor stopped replying. #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments <hr>This is an automatic backport of pull request cometbft#5324 done by [Mergify](https://mergify.com). Co-authored-by: Alex | Interchain Labs <alex@cosmoslabs.io> Co-authored-by: arsushi <richie@asymmetric.re> Co-authored-by: Abdul Malek <me@almk.dev> Co-authored-by: Matt Acciai <matt@skip.money> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Tyler <48813565+technicallyty@users.noreply.github.com> Co-authored-by: maradini77 <140460067+maradini77@users.noreply.github.com>

…#5324) (cometbft#5407) --- Updates the consensus reactor to validate that a received proposal will not contain more parts than the amount of chunks that it would take to build a block whos size is equal to `ConsensusParams.Block.MaxBytes`. Original PR is here cometbft#5309, but reopened since the contributor stopped replying. - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments <hr>This is an automatic backport of pull request cometbft#5324 done by [Mergify](https://mergify.com). Co-authored-by: Alex | Interchain Labs <alex@cosmoslabs.io> Co-authored-by: arsushi <richie@asymmetric.re> Co-authored-by: Abdul Malek <me@almk.dev> Co-authored-by: Matt Acciai <matt@skip.money> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Tyler <48813565+technicallyty@users.noreply.github.com> Co-authored-by: maradini77 <140460067+maradini77@users.noreply.github.com>

@odeke-em

* fix(bits): prevent BitArray.UnmarshalJSON from crashing on 0 bits in the JSON (backport cometbft#2774) (cometbft#2778) This change fixes a bug in which BitArray.UnmarshalJSON hadn't accounted for the fact that invoking NewBitArray(<=0) returns nil and hence when dereferenced would crash with a runtime nil pointer dereference. This bug was found by my security analysis and fuzzing too. Author: @odeke-em Fixes cometbft#2658 --- - [x] Tests written/updated - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] ~~Updated relevant documentation (`docs/` or `spec/`) and code comments~~ - [x] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec <hr>This is an automatic backport of pull request cometbft#2774 done by [Mergify](https://mergify.com). --------- Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com> * fix(consensus/reactor): reject oversized proposals (backport cometbft#5324) (cometbft#5407) --- Updates the consensus reactor to validate that a received proposal will not contain more parts than the amount of chunks that it would take to build a block whos size is equal to `ConsensusParams.Block.MaxBytes`. Original PR is here cometbft#5309, but reopened since the contributor stopped replying. - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments <hr>This is an automatic backport of pull request cometbft#5324 done by [Mergify](https://mergify.com). Co-authored-by: Alex | Interchain Labs <alex@cosmoslabs.io> Co-authored-by: arsushi <richie@asymmetric.re> Co-authored-by: Abdul Malek <me@almk.dev> Co-authored-by: Matt Acciai <matt@skip.money> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Tyler <48813565+technicallyty@users.noreply.github.com> Co-authored-by: maradini77 <140460067+maradini77@users.noreply.github.com> * Merge commit from fork * add VaidateBasic to BitArray to ensure Bits and len(Elems) are valid * call ValidateBasic on BitArrays when receiving as a msg from exteranl nodes * enfore SetIndex is not setting out of bounds * add guard to getNumTrueIndices getNumTrueIndices will index out of bounds if Bits and Elems have a mismatch where len(elems) != (bits+63)/64, this guard makes it simply return 0 if this mismatch is present * changelog * fix missing import for v0.38.x * update changelog for release of v0.38.19 * remove duplicate bug fixes from unreleased * fix changelog date * fix lint * fix expected error string in test * add necessary test constants --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com> Co-authored-by: Alex | Interchain Labs <alex@cosmoslabs.io> Co-authored-by: arsushi <richie@asymmetric.re> Co-authored-by: Abdul Malek <me@almk.dev> Co-authored-by: Matt Acciai <matt@skip.money> Co-authored-by: Tyler <48813565+technicallyty@users.noreply.github.com> Co-authored-by: maradini77 <140460067+maradini77@users.noreply.github.com> Co-authored-by: Matt Acciai <matt@cosmoslabs.io>

…#5324) (cometbft#5407) --- Updates the consensus reactor to validate that a received proposal will not contain more parts than the amount of chunks that it would take to build a block whos size is equal to `ConsensusParams.Block.MaxBytes`. Original PR is here cometbft#5309, but reopened since the contributor stopped replying. #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments <hr>This is an automatic backport of pull request cometbft#5324 done by [Mergify](https://mergify.com). Co-authored-by: Alex | Interchain Labs <alex@cosmoslabs.io> Co-authored-by: arsushi <richie@asymmetric.re> Co-authored-by: Abdul Malek <me@almk.dev> Co-authored-by: Matt Acciai <matt@skip.money> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Tyler <48813565+technicallyty@users.noreply.github.com> Co-authored-by: maradini77 <140460067+maradini77@users.noreply.github.com>

--- Updates the consensus reactor to validate that a received proposal will not contain more parts than the amount of chunks that it would take to build a block whos size is equal to `ConsensusParams.Block.MaxBytes`. Original PR is here #5309, but reopened since the contributor stopped replying. #### PR checklist - [ ] Tests written/updated - [ ] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments --------- Co-authored-by: arsushi <richie@asymmetric.re> Co-authored-by: Abdul Malek <me@almk.dev> Co-authored-by: Matt Acciai <matt@skip.money> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Tyler <48813565+technicallyty@users.noreply.github.com> Co-authored-by: maradini77 <140460067+maradini77@users.noreply.github.com>

arsushi and others added 3 commits August 25, 2025 22:17

reject oversized proposals

88e9ea6

Update internal/consensus/reactor.go

621956b

Co-authored-by: Abdul Malek <me@almk.dev>

add TestRejectOversizedProposals test

6639886

aljo242 requested review from a team as code owners September 2, 2025 18:04

mattac21 added 2 commits September 15, 2025 16:50

Merge branch 'main' into arsushi/main

2576ed5

TestRejectOversizedProposals properly assets that an oversized propos…

ff19cf8

…al is not accepted by a peer

mattac21 force-pushed the arsushi/main branch from 17468b9 to ff19cf8 Compare September 15, 2025 21:47

mattac21 requested review from Eric-Warehime, mattac21 and technicallyty as code owners September 15, 2025 21:47

mattac21 changed the title ~~fix(internal/consensus/reactor.go): reject oversized proposals~~ fix(consensus/reactor): reject oversized proposals Sep 16, 2025

defer stop on test consensus reactors

bf39875

This comment was marked as outdated.

Sign in to view

mattac21 added 2 commits September 16, 2025 11:45

require no error when stopping reactors

dcbbf58

rlock state when accessing block max bytes consensus param

8ac81f7

mattac21 added backport-to-v0.37.x backport-to-v0.38.x Tell Mergify to backport the PR to v0.38.x and removed backport-to-v0.37.x labels Sep 16, 2025

mattac21 and others added 2 commits September 16, 2025 12:13

changelog

692ec31

Merge branch 'main' into arsushi/main

c395c66

almk-dev approved these changes Sep 16, 2025

View reviewed changes

consensus/reactor.go Show resolved Hide resolved

mergify bot and others added 4 commits September 17, 2025 14:34

Merge branch 'main' into arsushi/main

729bc76

refactor proposal validation into ValidateBlockSize function

62cf7cb

mattac21 force-pushed the arsushi/main branch from 302a1d8 to 62cf7cb Compare September 17, 2025 21:12

Merge branch 'main' into arsushi/main

e5ab2cd

aljo242 commented Sep 17, 2025

View reviewed changes

types/proposal.go Show resolved Hide resolved

add test for proposal validate block size

87b9b3b

aljo242 merged commit 9ff47d0 into main Sep 17, 2025
32 checks passed

aljo242 deleted the arsushi/main branch September 17, 2025 21:54

mergify bot mentioned this pull request Sep 17, 2025

fix(consensus/reactor): reject oversized proposals (backport #5324) #5407

Merged

3 tasks

fridrik01 mentioned this pull request Jan 14, 2026

fix(consensus/reactor): reject oversized proposals berachain/cometbft#37

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(consensus/reactor): reject oversized proposals#5324

fix(consensus/reactor): reject oversized proposals#5324
aljo242 merged 16 commits intomainfrom
arsushi/main

aljo242 commented Sep 2, 2025 •

edited by mattac21

Loading

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

aljo242 commented Sep 2, 2025 • edited by mattac21 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR checklist

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

aljo242 commented Sep 2, 2025 •

edited by mattac21

Loading