From d7c35639c91c462a47f6502a54eada86c6345923 Mon Sep 17 00:00:00 2001 From: vagola Date: Sat, 4 Jan 2020 12:39:31 +0300 Subject: [PATCH] Validate string max length --- protocol-derive/src/lib.rs | 14 +++++++++++ protocol/src/game.rs | 48 ++++++++++++++++++++++++++++++++++++-- protocol/src/lib.rs | 22 ++++++++--------- 3 files changed, 71 insertions(+), 13 deletions(-) diff --git a/protocol-derive/src/lib.rs b/protocol-derive/src/lib.rs index ec99738..2953b8f 100644 --- a/protocol-derive/src/lib.rs +++ b/protocol-derive/src/lib.rs @@ -37,6 +37,13 @@ fn impl_encoder_trait(name: &Ident, fields: &Fields) -> TokenStream2 { let unparsed_meta = get_packet_field_meta(field); let parsed_meta = parse_packet_field_meta(&unparsed_meta); + // This is special case because max length are used only for strings. + if let Some(max_length) = parsed_meta.max_length { + return quote! { + crate::EncoderWriteExt::write_string(writer, &self.#name, #max_length)?; + }; + } + let module = parsed_meta.module.as_deref().unwrap_or("Encoder"); let module_ident = Ident::new(&module, Span::call_site()); @@ -65,6 +72,13 @@ fn impl_decoder_trait(name: &Ident, fields: &Fields) -> TokenStream2 { let unparsed_meta = get_packet_field_meta(field); let parsed_meta = parse_packet_field_meta(&unparsed_meta); + // This is special case because max length are used only for strings. + if let Some(max_length) = parsed_meta.max_length { + return quote! { + let #name = crate::DecoderReadExt::read_string(reader, #max_length)?; + }; + } + match parsed_meta.module { Some(module) => { let module_ident = Ident::new(&module, Span::call_site()); diff --git a/protocol/src/game.rs b/protocol/src/game.rs index 44afedb..3261e38 100644 --- a/protocol/src/game.rs +++ b/protocol/src/game.rs @@ -236,8 +236,8 @@ mod tests { ChunkData, ClientBoundChatMessage, ClientBoundKeepAlive, GameMode, JoinGame, MessagePosition, ServerBoundChatMessage, ServerBoundKeepAlive, }; - use crate::Decoder; - use crate::Encoder; + use crate::{DecodeError, Encoder, EncoderWriteExt, STRING_MAX_LENGTH}; + use crate::{Decoder, EncodeError}; use nbt::CompoundTag; use std::io::Cursor; @@ -266,6 +266,50 @@ mod tests { assert_eq!(chat_message.message, "hello server!"); } + #[test] + fn test_server_bound_chat_message_encode_invalid_length() { + let chat_message = ServerBoundChatMessage { + message: "abc".repeat(100), + }; + + let mut vec = Vec::new(); + + let encode_error = chat_message + .encode(&mut vec) + .err() + .expect("Expected error `StringTooLong` because message has invalid length"); + + match encode_error { + EncodeError::StringTooLong { length, max_length } => { + assert_eq!(length, 300); + assert_eq!(max_length, 256); + } + _ => panic!("Expected `StringTooLong` but got `{:?}`", encode_error), + } + } + + #[test] + fn test_server_bound_chat_message_decode_invalid_length() { + let message = "abc".repeat(100); + + let mut vec = Vec::new(); + vec.write_string(&message, STRING_MAX_LENGTH).unwrap(); + + let mut cursor = Cursor::new(vec); + + let decode_error = ServerBoundChatMessage::decode(&mut cursor) + .err() + .expect("Expected error `StringTooLong` because message has invalid length"); + + match decode_error { + DecodeError::StringTooLong { length, max_length } => { + assert_eq!(length, 300); + assert_eq!(max_length, 256); + } + _ => panic!("Expected `StringTooLong` but got `{:?}`", decode_error), + } + } + #[test] fn test_client_bound_chat_message_encode() { let chat_message = ClientBoundChatMessage { diff --git a/protocol/src/lib.rs b/protocol/src/lib.rs index 30dd730..5024b31 100644 --- a/protocol/src/lib.rs +++ b/protocol/src/lib.rs @@ -25,8 +25,8 @@ pub mod status; /// Current supported protocol version. pub const PROTOCOL_VERSION: u32 = 498; /// Protocol limits maximum string length. -const STRING_MAX_LENGTH: u32 = 32_768; -const HYPHENATED_UUID_LENGTH: u32 = 36; +const STRING_MAX_LENGTH: u16 = 32_768; +const HYPHENATED_UUID_LENGTH: u16 = 36; /// Possible errors while encoding packet. #[derive(Debug)] @@ -36,7 +36,7 @@ pub enum EncodeError { /// String length. length: usize, /// Max string length. - max_length: u32, + max_length: u16, }, IOError { io_error: IoError, @@ -68,9 +68,9 @@ pub enum DecodeError { /// String length can't be more than provided value. StringTooLong { /// String length. - length: u32, + length: usize, /// Max string length. - max_length: u32, + max_length: u16, }, IOError { io_error: IoError, @@ -140,7 +140,7 @@ trait Decoder { trait EncoderWriteExt { fn write_bool(&mut self, value: bool) -> Result<(), EncodeError>; - fn write_string(&mut self, value: &str, max_length: u32) -> Result<(), EncodeError>; + fn write_string(&mut self, value: &str, max_length: u16) -> Result<(), EncodeError>; fn write_byte_array(&mut self, value: &[u8]) -> Result<(), EncodeError>; @@ -155,7 +155,7 @@ trait EncoderWriteExt { trait DecoderReadExt { fn read_bool(&mut self) -> Result; - fn read_string(&mut self, max_length: u32) -> Result; + fn read_string(&mut self, max_length: u16) -> Result; fn read_byte_array(&mut self) -> Result, DecodeError>; @@ -177,7 +177,7 @@ impl EncoderWriteExt for W { Ok(()) } - fn write_string(&mut self, value: &str, max_length: u32) -> Result<(), EncodeError> { + fn write_string(&mut self, value: &str, max_length: u16) -> Result<(), EncodeError> { let length = value.len(); if length > max_length as usize { @@ -224,10 +224,10 @@ impl DecoderReadExt for R { } } - fn read_string(&mut self, max_length: u32) -> Result { - let length = self.read_var_i32()? as u32; + fn read_string(&mut self, max_length: u16) -> Result { + let length = self.read_var_i32()? as usize; - if length > max_length as u32 { + if length as u16 > max_length { return Err(DecodeError::StringTooLong { length, max_length }); }