0xMiden / 0xMiden/protocol

bug: BlockSignatures::Deserializable skips TooManySignatures validation in read_from

Open
#3,494 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
132
Forks
167
Avg merge
1d 23h
Merged PRs (30d)
110

Description

### Packages versions

miden-protocol: v0.16.0-rc.2 (main branch, commit bd52b31)

### Bug description

`BlockSignatures::read_from` constructs the struct directly without calling `new()`, so the `TooManySignatures` length check (`signatures.len() > ValidatorKeys::MAX`) is skipped during deserialization.

**`new()` validates:**

```rust
pub fn new(signatures: Vec) -> Result {
if signatures.len() > ValidatorKeys::MAX {
return Err(BlockSignaturesError::TooManySignatures { count: signatures.len() });
}
Ok(Self { signatures })
}
```

**`read_from` skips validation:**

```rust
impl Deserializable for BlockSignatures {
fn read_from(source: &mut R) -> Result {
let signatures = Vec::::read_from(source)?;
Ok(Self { signatures }) // ← no new(), no MAX check
}
}
```

`ValidatorKeys::read_from` in the same codebase does this correctly by routing through `Self::new().map_err(...)`:

```rust
impl Deserializable for ValidatorKeys {
fn read_from(source: &mut R) -> Result {
let keys = Vec::::read_from(source)?;
Self::new(keys).map_err(|err| DeserializationError::InvalidValue(err.to_string()))
}
}
```

**Suggested fix:** Route `read_from` through `new()`, matching the `ValidatorKeys` pattern:

```rust
impl Deserializable for BlockSignatures {
fn read_from(source: &mut R) -> Result {
let signatures = Vec::::read_from(source)?;
Self::new(signatures).map_err(|err| DeserializationError::InvalidValue(err.to_string()))
}
}
```

Happy to open a PR if this direction is agreed upon.

**Related:**
- #3185 same pattern in `ProposedBlock::read_from` (skips `new_at` validation)
- #3199 same pattern in `SignedBlock::read_from` (skips signature verification)

### How can this be reproduced?

1. Serialize a `BlockSignatures` with more than `ValidatorKeys::MAX` (5) signatures
2. Deserialize with `read_from_bytes()`
3. Deserialization succeeds `TooManySignatures` validation is bypassed
4. `BlockSignatures::new()` with the same data would return `Err(TooManySignatures { count: 6 })`

### Relevant log output

```shell
No logs structural code issue.
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.