From bc205d2683b7bfeb5b3150c31c607ad70ebad07b Mon Sep 17 00:00:00 2001 From: Alexander Medvedev Date: Sun, 22 Feb 2026 15:24:36 +0100 Subject: [PATCH] feat: pumpkin-protocol: add fuzzing actually found some issues --- Cargo.lock | 50 ++++++ Cargo.toml | 3 +- pumpkin-protocol/fuzz/.gitignore | 4 + pumpkin-protocol/fuzz/Cargo.toml | 25 +++ .../fuzz/fuzz_targets/decoder_java.rs | 147 ++++++++++++++++++ pumpkin-protocol/src/codec/data_component.rs | 13 +- .../src/codec/item_stack_seralizer.rs | 120 ++++++++------ pumpkin-protocol/src/ser/mod.rs | 17 +- 8 files changed, 323 insertions(+), 56 deletions(-) create mode 100644 pumpkin-protocol/fuzz/.gitignore create mode 100644 pumpkin-protocol/fuzz/Cargo.toml create mode 100644 pumpkin-protocol/fuzz/fuzz_targets/decoder_java.rs diff --git a/Cargo.lock b/Cargo.lock index e7295d771..16dd0ca71 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -55,6 +55,12 @@ version = "1.0.102" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" +[[package]] +name = "arbitrary" +version = "1.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c3d036a3c4ab069c7b410a2ce876bd74808d2d0888a82667669f8e783a898bf1" + [[package]] name = "arc-swap" version = "1.8.2" @@ -227,6 +233,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "aebf35691d1bfb0ac386a69bac2fde4dd276fb618cf8bf4f5318fe285e821bb2" dependencies = [ "find-msvc-tools", + "jobserver", + "libc", "shlex", ] @@ -1011,6 +1019,18 @@ dependencies = [ "wasi", ] +[[package]] +name = "getrandom" +version = "0.3.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "899def5c37c4fd7b2664648c28120ecec138e4d395b459e5ca34f9cce2dd77fd" +dependencies = [ + "cfg-if", + "libc", + "r-efi", + "wasip2", +] + [[package]] name = "getrandom" version = "0.4.1" @@ -1417,6 +1437,16 @@ version = "1.0.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "92ecc6618181def0457392ccd0ee51198e065e016d1d527a7ac1b6dc7c1f09d2" +[[package]] +name = "jobserver" +version = "0.1.34" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9afb3de4395d6b3e67a780b6de64b51c978ecf11cb9a462c66be7d4ca9039d33" +dependencies = [ + "getrandom 0.3.4", + "libc", +] + [[package]] name = "js-sys" version = "0.3.87" @@ -1445,6 +1475,16 @@ version = "0.2.182" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6800badb6cb2082ffd7b6a67e6125bb39f18782f793520caee8cb8846be06112" +[[package]] +name = "libfuzzer-sys" +version = "0.4.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f12a681b7dd8ce12bff52488013ba614b869148d54dd79836ab85aafdd53f08d" +dependencies = [ + "arbitrary", + "cc", +] + [[package]] name = "libloading" version = "0.9.0" @@ -2141,6 +2181,15 @@ dependencies = [ "uuid", ] +[[package]] +name = "pumpkin-protocol-fuzz" +version = "0.0.0" +dependencies = [ + "libfuzzer-sys", + "pumpkin-protocol", + "tokio", +] + [[package]] name = "pumpkin-util" version = "0.1.0-dev+1.21.11" @@ -2851,6 +2900,7 @@ dependencies = [ "bytes", "libc", "mio", + "parking_lot", "pin-project-lite", "signal-hook-registry", "socket2", diff --git a/Cargo.toml b/Cargo.toml index ef3b88ac1..dfbe5b3e3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,6 +7,7 @@ members = [ "pumpkin-inventory", "pumpkin-macros/", "pumpkin-protocol/", + "pumpkin-protocol/fuzz", "pumpkin-world", "pumpkin/", "pumpkin-data", @@ -139,7 +140,7 @@ phf = "0.13.1" pkcs8 = "=0.11.0-rc.11" proc-macro2 = "1.0" pumpkin-config = { path = "pumpkin-config" } -pumpkin-data = { path = "pumpkin-data", default-features = false } +pumpkin-data = { path = "pumpkin-data" } pumpkin-inventory = { path = "pumpkin-inventory" } pumpkin-macros = { path = "pumpkin-macros" } pumpkin-nbt = { path = "pumpkin-nbt" } diff --git a/pumpkin-protocol/fuzz/.gitignore b/pumpkin-protocol/fuzz/.gitignore new file mode 100644 index 000000000..1a45eee77 --- /dev/null +++ b/pumpkin-protocol/fuzz/.gitignore @@ -0,0 +1,4 @@ +target +corpus +artifacts +coverage diff --git a/pumpkin-protocol/fuzz/Cargo.toml b/pumpkin-protocol/fuzz/Cargo.toml new file mode 100644 index 000000000..64a61b34b --- /dev/null +++ b/pumpkin-protocol/fuzz/Cargo.toml @@ -0,0 +1,25 @@ +[package] +name = "pumpkin-protocol-fuzz" +version = "0.0.0" +publish = false +edition = "2024" + +# IMPORTANT: disable lto when running +# CARGO_PROFILE_RELEASE_LTO=false cargo +nightly fuzz run decoder + +[package.metadata] +cargo-fuzz = true + +[dependencies] +tokio = { version = "1", features = ["full"] } +libfuzzer-sys = "0.4" + +[dependencies.pumpkin-protocol] +path = ".." + +[[bin]] +name = "decoder_java" +path = "fuzz_targets/decoder_java.rs" +test = false +doc = false +bench = false diff --git a/pumpkin-protocol/fuzz/fuzz_targets/decoder_java.rs b/pumpkin-protocol/fuzz/fuzz_targets/decoder_java.rs new file mode 100644 index 000000000..f8bd5a28a --- /dev/null +++ b/pumpkin-protocol/fuzz/fuzz_targets/decoder_java.rs @@ -0,0 +1,147 @@ +#![no_main] +use libfuzzer_sys::fuzz_target; +use pumpkin_protocol::{ + ServerPacket, + java::{ + packet_decoder::TCPNetworkDecoder, + server::{ + config::{ + SClientInformationConfig, SConfigCookieResponse, SConfigResourcePack, SKnownPacks, + SPluginMessage, + }, + handshake::SHandShake, + login::{SEncryptionResponse, SLoginCookieResponse, SLoginPluginResponse, SLoginStart}, + play::{ + SChangeGameMode, SChatCommand, SChatMessage, SChunkBatch, SClickSlot, + SClientCommand, SClientInformationPlay, SCloseContainer, SCommandSuggestion, + SConfirmTeleport, SCookieResponse, SCustomPayload, SInteract, SKeepAlive, + SMoveVehicle, SPaddleBoat, SPickItemFromBlock, SPlayPingRequest, SPlayerAbilities, + SPlayerAction, SPlayerCommand, SPlayerInput, SPlayerLoaded, SPlayerPosition, + SPlayerPositionRotation, SPlayerRotation, SPlayerSession, SSetCommandBlock, + SSetCreativeSlot, SSetHeldItem, SSetPlayerGround, SSwingArm, SUpdateSign, SUseItem, + SUseItemOn, + }, + status::SStatusPingRequest, + }, + }, +}; +use std::io::Cursor; +use tokio::runtime::Runtime; + +// --------------------------------------------------------------------------- +// Helper: run every known ServerPacket::read against the same payload. +// We don't care about the result — only that nothing panics. +// --------------------------------------------------------------------------- +fn fuzz_all_deserializers(payload: &[u8]) { + // Handshake + let _ = SHandShake::read(payload); + + // Status + //let _ = SStatusRequest::read(payload); + let _ = SStatusPingRequest::read(payload); + + // Login + let _ = SLoginStart::read(payload); + let _ = SEncryptionResponse::read(payload); + let _ = SLoginPluginResponse::read(payload); + //let _ = SLoginAcknowledged::read(payload); + let _ = SLoginCookieResponse::read(payload); + + // Config + let _ = SClientInformationConfig::read(payload); + let _ = SPluginMessage::read(payload); + //let _ = SAcknowledgeFinishConfig::read(payload); + let _ = SKnownPacks::read(payload); + let _ = SConfigCookieResponse::read(payload); + let _ = SConfigResourcePack::read(payload); + + // Play + let _ = SConfirmTeleport::read(payload); + let _ = SChangeGameMode::read(payload); + let _ = SChatCommand::read(payload); + let _ = SChatMessage::read(payload); + let _ = SClientInformationPlay::read(payload); + let _ = SClientCommand::read(payload); + let _ = SPlayerInput::read(payload); + let _ = SMoveVehicle::read(payload); + let _ = SPaddleBoat::read(payload); + let _ = SInteract::read(payload); + let _ = SKeepAlive::read(payload); + let _ = SPlayerPosition::read(payload); + let _ = SPlayerPositionRotation::read(payload); + let _ = SPlayerRotation::read(payload); + let _ = SSetPlayerGround::read(payload); + let _ = SPickItemFromBlock::read(payload); + let _ = SPlayerAbilities::read(payload); + let _ = SPlayerAction::read(payload); + let _ = SSetCommandBlock::read(payload); + let _ = SPlayerCommand::read(payload); + let _ = SPlayerLoaded::read(payload); + let _ = SPlayPingRequest::read(payload); + let _ = SClickSlot::read(payload); + let _ = SSetHeldItem::read(payload); + let _ = SSetCreativeSlot::read(payload); + let _ = SSwingArm::read(payload); + let _ = SUpdateSign::read(payload); + let _ = SUseItemOn::read(payload); + let _ = SUseItem::read(payload); + let _ = SCommandSuggestion::read(payload); + let _ = SCookieResponse::read(payload); + let _ = SCloseContainer::read(payload); + let _ = SChunkBatch::read(payload); + let _ = SPlayerSession::read(payload); + let _ = SCustomPayload::read(payload); +} + +// --------------------------------------------------------------------------- +// Fuzz target +// --------------------------------------------------------------------------- +fuzz_target!(|data: &[u8]| { + if data.len() < 18 { + return; + } + + // Byte layout: + // [0] = decoder mode (0=plain, 1=compressed, 2=encrypted, 3=both) + // [1] = split point % rest.len() — how much goes to the decoder vs + // the deserializers, letting the fuzzer explore both paths + // independently with different byte patterns + // [2..18] = 16-byte AES key candidate + // [18..] = raw bytes fed to both fuzzing paths + let mode = data[0] % 4; + let key = &data[2..18]; + let rest = &data[18..]; + + let split = if rest.is_empty() { + 0 + } else { + (data[1] as usize) % rest.len() + }; + let (decoder_bytes, deser_bytes) = rest.split_at(split); + + // --- Path 1: decoder (framing / encryption / compression) -------------- + let rt = Runtime::new().unwrap(); + rt.block_on(async { + let mut decoder = TCPNetworkDecoder::new(Cursor::new(decoder_bytes)); + match mode { + 1 => { + decoder.set_compression(256); + } + 2 => { + let mut aes_key = [0u8; 16]; + aes_key.copy_from_slice(key); + decoder.set_encryption(&aes_key); + } + 3 => { + decoder.set_compression(256); + let mut aes_key = [0u8; 16]; + aes_key.copy_from_slice(key); + decoder.set_encryption(&aes_key); + } + _ => {} + } + let _ = decoder.get_raw_packet().await; + }); + + fuzz_all_deserializers(deser_bytes); +}); diff --git a/pumpkin-protocol/src/codec/data_component.rs b/pumpkin-protocol/src/codec/data_component.rs index a695de0d5..91ba9880a 100644 --- a/pumpkin-protocol/src/codec/data_component.rs +++ b/pumpkin-protocol/src/codec/data_component.rs @@ -54,10 +54,15 @@ impl DataComponentCodec for EnchantmentsImpl { Ok(()) } fn deserialize<'a, A: SeqAccess<'a>>(seq: &mut A) -> Result { + const MAX_ENCHANTMENTS: usize = 256; + let len = seq .next_element::()? .ok_or(de::Error::custom("No EnchantmentsImpl len VarInt!"))? .0 as usize; + if len > MAX_ENCHANTMENTS { + return Err(de::Error::custom("Too many enchantments")); + } let mut enc = Vec::with_capacity(len); for _ in 0..len { let id = seq @@ -134,6 +139,8 @@ impl DataComponentCodec for PotionContentsImpl { } fn deserialize<'a, A: SeqAccess<'a>>(seq: &mut A) -> Result { + const MAX_EFFECTS: usize = 128; + // Potion ID (optional) let has_potion = seq .next_element::()? @@ -166,7 +173,9 @@ impl DataComponentCodec for PotionContentsImpl { .next_element::()? .ok_or(de::Error::custom("No PotionContents effects_len VarInt!"))? .0 as usize; - + if effects_len > MAX_EFFECTS { + return Err(de::Error::custom("Too many potion effects")); + } let mut custom_effects = Vec::with_capacity(effects_len); for _ in 0..effects_len { let effect_id = seq @@ -423,7 +432,7 @@ pub fn deserialize<'a, A: SeqAccess<'a>>( DataComponent::PotionContents => Ok(PotionContentsImpl::deserialize(seq)?.to_dyn()), DataComponent::FireworkExplosion => Ok(FireworkExplosionImpl::deserialize(seq)?.to_dyn()), DataComponent::Fireworks => Ok(FireworksImpl::deserialize(seq)?.to_dyn()), - _ => todo!("{} not yet implemented", id.to_name()), + _ => Err(serde::de::Error::custom("TODO")), } } pub fn serialize( diff --git a/pumpkin-protocol/src/codec/item_stack_seralizer.rs b/pumpkin-protocol/src/codec/item_stack_seralizer.rs index 79e843052..c0bf965e9 100644 --- a/pumpkin-protocol/src/codec/item_stack_seralizer.rs +++ b/pumpkin-protocol/src/codec/item_stack_seralizer.rs @@ -73,70 +73,80 @@ impl<'de> Deserialize<'de> for ItemStackSerializer<'static> { } fn visit_seq>(self, mut seq: A) -> Result { + const MAX_COMPONENTS: i32 = 256; + let item_count = seq .next_element::()? - .ok_or(de::Error::custom("Failed to decode VarInt"))?; + .ok_or_else(|| de::Error::custom("Failed to decode VarInt"))?; - let slot = if item_count.0 == 0 { - ItemStackSerializer(Cow::Borrowed(ItemStack::EMPTY)) - } else { - let item_id = seq + if item_count.0 == 0 { + return Ok(ItemStackSerializer(Cow::Borrowed(ItemStack::EMPTY))); + } + + let item_id = seq + .next_element::()? + .ok_or_else(|| de::Error::custom("No item id VarInt!"))?; + + let num_to_add = seq.next_element::()?.map_or(0, |v| v.0); + let num_to_remove = seq.next_element::()?.map_or(0, |v| v.0); + + if num_to_add < 0 || num_to_remove < 0 { + return Err(de::Error::custom("Negative component count")); + } + + let total_components = num_to_add + .checked_add(num_to_remove) + .ok_or_else(|| de::Error::custom("Component count overflow"))?; + + if total_components > MAX_COMPONENTS { + return Err(de::Error::custom("Too many components in ItemStack patch")); + } + + let mut patch = Vec::with_capacity((num_to_add + num_to_remove) as usize); + + for _ in 0..num_to_add { + let id_val = seq .next_element::()? - .ok_or(de::Error::custom("No item id VarInt!"))?; + .ok_or_else(|| de::Error::custom("Missing component ID"))? + .0; + let id = DataComponent::try_from_id(id_val as u8).ok_or_else(|| { + de::Error::custom(format!("Unknown component ID: {id_val}")) + })?; - let num_components_to_add = seq + // Minecraft protocol sends a byte length for the component data here + let _byte_len = seq .next_element::()? - .ok_or(de::Error::custom("No component add length VarInt!"))? - .0 as usize; - let num_components_to_remove = seq + .ok_or_else(|| de::Error::custom("No data len VarInt!"))?; + + let component_impl = deserialize(id, &mut seq)?; + + patch.push((id, Some(component_impl))); + } + + for _ in 0..num_to_remove { + let id_val = seq .next_element::()? - .ok_or(de::Error::custom("No component remove length VarInt!"))? - .0 as usize; + .ok_or_else(|| de::Error::custom("Missing remove component ID"))? + .0; + let id = DataComponent::try_from_id(id_val as u8) + .ok_or_else(|| de::Error::custom("Unknown component ID"))?; + patch.push((id, None)); + } - let mut patch = - Vec::with_capacity(num_components_to_add + num_components_to_remove); - for _ in 0..num_components_to_add { - let id = seq - .next_element::()? - .ok_or(de::Error::custom("No component id VarInt!"))? - .0; - let id = u8::try_from(id) - .map_err(|_| de::Error::custom("Unknown component id VarInt!"))?; - let id = DataComponent::try_from_id(id) - .ok_or(de::Error::custom("Unknown component id VarInt!"))?; - let _byte_len = seq - .next_element::()? - .ok_or(de::Error::custom("No data len VarInt!"))?; - patch.push((id, Some(deserialize(id, &mut seq)?))); - } - for _ in 0..num_components_to_remove { - let id = seq - .next_element::()? - .ok_or(de::Error::custom("No component id VarInt!"))? - .0; - let id = u8::try_from(id) - .map_err(|_| de::Error::custom("Unknown component id VarInt!"))?; - let id = DataComponent::try_from_id(id) - .ok_or(de::Error::custom("Unknown component id VarInt!"))?; - patch.push((id, None)); - } + let item_id_u16: u16 = item_id + .0 + .try_into() + .map_err(|_| de::Error::custom("Invalid item id!"))?; - let item_id: u16 = item_id - .0 - .try_into() - .map_err(|_| de::Error::custom("Invalid item id!"))?; - - ItemStackSerializer(Cow::Owned(ItemStack::new_with_component( + Ok(ItemStackSerializer(Cow::Owned( + ItemStack::new_with_component( item_count.0 as u8, - Item::from_id(item_id).unwrap_or(&Item::AIR), + Item::from_id(item_id_u16).unwrap_or(&Item::AIR), patch, - ))) - }; - - Ok(slot) + ), + ))) } } - deserializer.deserialize_seq(Visitor) } } @@ -311,12 +321,17 @@ impl<'de> Deserialize<'de> for ItemComponentHash { } fn visit_seq>(self, mut seq: A) -> Result { + const MAX_COMPONENTS: i32 = 256; + let mut added = Vec::new(); let mut removed = Vec::new(); let added_length = seq .next_element::()? .ok_or(de::Error::custom("No added length VarInt!"))?; + if added_length.0 < 0 || added_length.0 > MAX_COMPONENTS { + return Err(de::Error::custom("added_length out of bounds")); + } for _ in 0..added_length.0 { let component_id = seq .next_element::()? @@ -330,6 +345,9 @@ impl<'de> Deserialize<'de> for ItemComponentHash { let removed_length = seq .next_element::()? .ok_or(de::Error::custom("No removed length VarInt!"))?; + if removed_length.0 < 0 || removed_length.0 > MAX_COMPONENTS { + return Err(de::Error::custom("removed_length out of bounds")); + } for _ in 0..removed_length.0 { let component_id = seq .next_element::()? diff --git a/pumpkin-protocol/src/ser/mod.rs b/pumpkin-protocol/src/ser/mod.rs index df8150f7c..1bab6dd89 100644 --- a/pumpkin-protocol/src/ser/mod.rs +++ b/pumpkin-protocol/src/ser/mod.rs @@ -101,8 +101,14 @@ impl NetworkReadExt for R { get_number_be!(get_f32_be, f32); get_number_be!(get_f64_be, f64); - fn read_boxed_slice(&mut self, count: usize) -> Result, ReadingError> { - let mut buf = vec![0u8; count]; + fn read_boxed_slice(&mut self, length: usize) -> Result, ReadingError> { + const MAX_SLICE_LENGTH: usize = 2 * 1024 * 64; // 64KB, largest valid MC packet + if !(1..=MAX_SLICE_LENGTH).contains(&length) { + return Err(ReadingError::Message(format!( + "read_boxed_slice: length {length} out of bounds" + ))); + } + let mut buf = vec![0u8; length]; self.read_exact(&mut buf) .map_err(|err| ReadingError::Incomplete(err.to_string()))?; @@ -187,7 +193,14 @@ impl NetworkReadExt for R { &mut self, parse: impl Fn(&mut Self) -> Result, ) -> Result, ReadingError> { + const MAX_LIST_SIZE: usize = 65536; + let len = self.get_var_int()?.0 as usize; + if len > MAX_LIST_SIZE { + return Err(ReadingError::TooLarge(format!( + "List length {len} exceeds limit" + ))); + } let mut list = Vec::with_capacity(len); for _ in 0..len { list.push(parse(self)?);