Skip to content

Commit d5faea1

Browse files
committed
fix(sync): bound entry key size to prevent replica memory/disk exhaustion
Sync-peer-supplied entries were accepted with arbitrarily large keys, bounded only by the codec's MAX_MESSAGE_SIZE (1 GiB). `validate_entry` did not check key length, so a peer that opened a sync session could push entries with huge keys and force every replica that synced them to persist them — a peer-controllable memory and disk amplifier. Note: the key-size check has to live in `validate_entry` (or an equivalent post-deserialization hook), not in `RecordIdentifier::new`, because `RecordIdentifier` derives `Deserialize` and peer-supplied entries reach the replica via serde, bypassing the constructor entirely. Fix: add `MAX_ENTRY_KEY_SIZE = 4096` and reject any entry whose key exceeds it with a new `ValidationFailure::KeyTooLarge { actual, max }`. The check runs before signature verification so oversized entries are rejected without spending crypto work. 4 KiB is deliberately generous for legitimate use cases (keys are short identifiers / paths); tunable if upstream prefers a different limit. Reproduction: the added `test_peer_entry_with_oversized_key_rejected` test builds a 1 MiB key into a `SignedEntry` and feeds it through `insert_remote_entry`. Before the bound the test's predecessor (`test_peer_entry_with_oversized_key_accepted`, run against upstream) confirmed the entry was accepted. With the bound in place the test asserts `KeyTooLarge` is returned, and that a key at exactly `MAX_ENTRY_KEY_SIZE` is still accepted.
1 parent 722c200 commit d5faea1

1 file changed

Lines changed: 82 additions & 0 deletions

File tree

src/sync.rs

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -603,9 +603,19 @@ where
603603
#[error("Replica allows read access only.")]
604604
pub struct ReadOnly;
605605

606+
/// Maximum allowed length of an entry key, in bytes.
607+
///
608+
/// Entry keys are part of each [`SignedEntry`] and are therefore stored on
609+
/// every replica that syncs them. Without a bound a sync peer can submit
610+
/// entries with arbitrarily large keys (up to the codec's `MAX_MESSAGE_SIZE`,
611+
/// which is 1 GiB), causing memory / disk exhaustion on receivers. 4 KiB is
612+
/// deliberately generous; legitimate applications use short identifiers.
613+
pub const MAX_ENTRY_KEY_SIZE: usize = 4096;
614+
606615
/// Validate a [`SignedEntry`] if it's fit to be inserted.
607616
///
608617
/// This validates that
618+
/// * the entry's key is at most [`MAX_ENTRY_KEY_SIZE`] bytes
609619
/// * the entry's author and namespace signatures are correct
610620
/// * the entry's namespace matches the current replica
611621
/// * the entry's timestamp is not more than 10 minutes in the future of our system time
@@ -617,6 +627,15 @@ fn validate_entry<S: ranger::Store<SignedEntry> + PublicKeyStore>(
617627
entry: &SignedEntry,
618628
origin: &InsertOrigin,
619629
) -> Result<(), ValidationFailure> {
630+
// Check key size before signature verification so that oversized entries
631+
// from a peer are rejected cheaply, without spending crypto work.
632+
if entry.key().len() > MAX_ENTRY_KEY_SIZE {
633+
return Err(ValidationFailure::KeyTooLarge {
634+
actual: entry.key().len(),
635+
max: MAX_ENTRY_KEY_SIZE,
636+
});
637+
}
638+
620639
// Verify the namespace
621640
if entry.namespace() != expected_namespace {
622641
return Err(ValidationFailure::InvalidNamespace);
@@ -673,6 +692,14 @@ pub enum ValidationFailure {
673692
/// Entry has length 0 but not the empty hash, or the empty hash but not length 0.
674693
#[error("Entry has length 0 but not the empty hash, or the empty hash but not length 0")]
675694
InvalidEmptyEntry,
695+
/// Entry key exceeds the maximum allowed length.
696+
#[error("Entry key of {actual} bytes exceeds maximum of {max} bytes")]
697+
KeyTooLarge {
698+
/// The actual key length in bytes.
699+
actual: usize,
700+
/// The maximum allowed key length in bytes.
701+
max: usize,
702+
},
676703
}
677704

678705
/// A signed entry.
@@ -1781,6 +1808,61 @@ mod tests {
17811808
Ok(())
17821809
}
17831810

1811+
#[tokio::test]
1812+
async fn test_peer_entry_with_oversized_key_rejected() -> Result<()> {
1813+
// Regression test for unbounded entry key size on the sync-origin
1814+
// insert path.
1815+
//
1816+
// Before the [`MAX_ENTRY_KEY_SIZE`] bound in `validate_entry` a sync
1817+
// peer could submit a [`SignedEntry`] whose key was bounded only by
1818+
// the codec's MAX_MESSAGE_SIZE (1 GiB). The entry was then stored in
1819+
// every replica that synced it, giving a peer-controllable
1820+
// memory/disk amplifier.
1821+
//
1822+
// This test builds a 1 MiB key and feeds the resulting signed entry
1823+
// through `insert_remote_entry`, asserting that it is now rejected
1824+
// with [`ValidationFailure::KeyTooLarge`]. It also checks that a
1825+
// key at exactly the boundary is still accepted.
1826+
let mut store = store::Store::memory();
1827+
let mut rng = rand::rng();
1828+
let alice = Author::new(&mut rng);
1829+
let namespace = NamespaceSecret::new(&mut rng);
1830+
let peer_id: [u8; 32] = *alice.id().as_bytes();
1831+
1832+
// (1) 1 MiB key — should be rejected post-fix.
1833+
let huge_key = vec![0u8; 1024 * 1024];
1834+
let hash = Hash::new(b"payload");
1835+
let record = Record::new(hash, 7, system_time_now());
1836+
let entry = SignedEntry::from_parts(&namespace, &alice, &huge_key, record);
1837+
1838+
let mut replica = store.new_replica(namespace.clone())?;
1839+
let result = replica
1840+
.insert_remote_entry(entry, peer_id, ContentStatus::Missing)
1841+
.await;
1842+
assert!(
1843+
matches!(
1844+
result,
1845+
Err(InsertError::Validation(
1846+
ValidationFailure::KeyTooLarge { .. }
1847+
))
1848+
),
1849+
"expected KeyTooLarge rejection; got {:?}",
1850+
result
1851+
);
1852+
1853+
// (2) MAX_ENTRY_KEY_SIZE exactly — should be accepted.
1854+
let boundary_key = vec![b'x'; MAX_ENTRY_KEY_SIZE];
1855+
let record2 = Record::new(Hash::new(b"payload2"), 8, system_time_now());
1856+
let entry_ok = SignedEntry::from_parts(&namespace, &alice, &boundary_key, record2);
1857+
replica
1858+
.insert_remote_entry(entry_ok, peer_id, ContentStatus::Missing)
1859+
.await
1860+
.expect("boundary-sized key should be accepted");
1861+
1862+
store.flush()?;
1863+
Ok(())
1864+
}
1865+
17841866
#[tokio::test]
17851867
async fn test_insert_empty() -> Result<()> {
17861868
let mut store = store::Store::memory();

0 commit comments

Comments
 (0)