PartialStorage::read_from skips the map-root check that PartialStorage::new enforces
- Lenguaje dominante
- Rust
- Estrellas
- 132
- Forks
- 167
- Merge medio
- 1 d 23 h
- PR fusionados (30 d)
- 110
Descripción
Was looking through the account storage code and noticed something odd in `crates/miden-protocol/src/account/storage/partial.rs`.
`PartialStorage::new` checks that every `PartialStorageMap` passed in has a root matching one of the slot roots in the storage header, and returns `StorageMapRootNotFound` if not (this check itself was added/fixed in #1647). But `PartialStorage`'s `Deserializable::read_from` doesn't go through `new` at all, it just builds the struct straight from the deserialized fields:
```rust
impl Deserializable for PartialStorage {
fn read_from(source: &mut R) -> Result {
let header: AccountStorageHeader = source.read()?;
let map_smts: BTreeMap = source.read()?;
let commitment = header.to_commitment();
Ok(PartialStorage { header, maps: map_smts, commitment })
}
}
```
So if you deserialize bytes where a map's root doesn't line up with any slot root in the header, it just goes through, no error, even though `new()` would reject exactly this case. Same class of issue as #3494 (BlockSignatures): there's a validating constructor, but the Deserializable impl doesn't route through it, and in this case the root-check in `new()` was specifically added by #1647 without the deserialize path ever being updated to match.
I confirmed this locally with a test: constructed a `PartialStorage` bypassing `new()` (same shape `read_from` produces), round-tripped it through `to_bytes()` / `read_from_bytes()`, and it deserializes successfully with a map whose root isn't present in the header at all. `new()` correctly rejects the same data.
Happy to send a PR for this if it can be assigned to me — I already have a working regression test for it, the fix itself is just having `read_from` validate the roots the same way `new()` does.
Guía de contribución
Evaluación
Este issue todavía no se ha evaluado.