feat: pumpkin-protocol: add fuzzing

actually found some issues

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",

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" }

pumpkin-protocol/fuzz/.gitignore

@@ -0,0 +1,4 @@
+target
+corpus
+artifacts
+coverage

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

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);
+});

pumpkin-protocol/src/codec/data_component.rs

@@ -54,10 +54,15 @@ impl DataComponentCodec<Self> for EnchantmentsImpl {
         Ok(())
     }
     fn deserialize<'a, A: SeqAccess<'a>>(seq: &mut A) -> Result<Self, A::Error> {
+        const MAX_ENCHANTMENTS: usize = 256;
+
         let len = seq
             .next_element::<VarInt>()?
             .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<Self> for PotionContentsImpl {
     }
 
     fn deserialize<'a, A: SeqAccess<'a>>(seq: &mut A) -> Result<Self, A::Error> {
+        const MAX_EFFECTS: usize = 128;
+
         // Potion ID (optional)
         let has_potion = seq
             .next_element::<bool>()?
@@ -166,7 +173,9 @@ impl DataComponentCodec<Self> for PotionContentsImpl {
             .next_element::<VarInt>()?
             .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<T: SerializeStruct>(

pumpkin-protocol/src/codec/item_stack_seralizer.rs

@@ -73,70 +73,80 @@ impl<'de> Deserialize<'de> for ItemStackSerializer<'static> {
             }
 
             fn visit_seq<A: SeqAccess<'de>>(self, mut seq: A) -> Result<Self::Value, A::Error> {
+                const MAX_COMPONENTS: i32 = 256;
+
                 let item_count = seq
                     .next_element::<VarInt>()?
-                    .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 {
+                if item_count.0 == 0 {
-                    ItemStackSerializer(Cow::Borrowed(ItemStack::EMPTY))
+                    return Ok(ItemStackSerializer(Cow::Borrowed(ItemStack::EMPTY)));
-                } else {
+                }
-                    let item_id = seq
+
+                let item_id = seq
+                    .next_element::<VarInt>()?
+                    .ok_or_else(|| de::Error::custom("No item id VarInt!"))?;
+
+                let num_to_add = seq.next_element::<VarInt>()?.map_or(0, |v| v.0);
+                let num_to_remove = seq.next_element::<VarInt>()?.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::<VarInt>()?
-                        .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::<VarInt>()?
-                        .ok_or(de::Error::custom("No component add length VarInt!"))?
+                        .ok_or_else(|| de::Error::custom("No data len VarInt!"))?;
-                        .0 as usize;
+
-                    let num_components_to_remove = seq
+                    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::<VarInt>()?
-                        .ok_or(de::Error::custom("No component remove length VarInt!"))?
+                        .ok_or_else(|| de::Error::custom("Missing remove component ID"))?
-                        .0 as usize;
+                        .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 =
+                let item_id_u16: u16 = item_id
-                        Vec::with_capacity(num_components_to_add + num_components_to_remove);
+                    .0
-                    for _ in 0..num_components_to_add {
+                    .try_into()
-                        let id = seq
+                    .map_err(|_| de::Error::custom("Invalid item id!"))?;
-                            .next_element::<VarInt>()?
-                            .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::<VarInt>()?
-                            .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::<VarInt>()?
-                            .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 = item_id
+                Ok(ItemStackSerializer(Cow::Owned(
-                        .0
+                    ItemStack::new_with_component(
-                        .try_into()
-                        .map_err(|_| de::Error::custom("Invalid item id!"))?;
-
-                    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<A: SeqAccess<'de>>(self, mut seq: A) -> Result<Self::Value, A::Error> {
+                const MAX_COMPONENTS: i32 = 256;
+
                 let mut added = Vec::new();
                 let mut removed = Vec::new();
 
                 let added_length = seq
                     .next_element::<VarInt>()?
                     .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::<VarInt>()?
@@ -330,6 +345,9 @@ impl<'de> Deserialize<'de> for ItemComponentHash {
                 let removed_length = seq
                     .next_element::<VarInt>()?
                     .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::<VarInt>()?

pumpkin-protocol/src/ser/mod.rs

@@ -101,8 +101,14 @@ impl<R: Read> 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<Box<[u8]>, ReadingError> {
+    fn read_boxed_slice(&mut self, length: usize) -> Result<Box<[u8]>, ReadingError> {
-        let mut buf = vec![0u8; count];
+        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<R: Read> NetworkReadExt for R {
         &mut self,
         parse: impl Fn(&mut Self) -> Result<G, ReadingError>,
     ) -> Result<Vec<G>, 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)?);