From 34f4f99339a0baa7f535ec774d5c3e9ba4933939 Mon Sep 17 00:00:00 2001 From: Henri Chataing Date: Fri, 24 Jul 2026 14:53:40 +0000 Subject: [PATCH] Missing checks and test cases for invalid element size field --- pdl-compiler/src/backends/cxx.rs | 18 +++++++--- pdl-compiler/src/backends/rust/decoder.rs | 33 +++++++++++++++++ .../tests/canonical/be_test_vectors.json | 36 +++++++++++++++++++ .../tests/canonical/le_test_vectors.json | 16 +++++++++ pdl-compiler/tests/generated/cxx/be_backend.h | 10 ++++-- pdl-compiler/tests/generated/cxx/le_backend.h | 10 ++++-- ...l_array_dynamic_element_size_big_endian.rs | 7 ++++ ...c_element_size_dynamic_count_big_endian.rs | 7 ++++ ...lement_size_dynamic_count_little_endian.rs | 7 ++++ ...ic_element_size_dynamic_size_big_endian.rs | 7 ++++ ...element_size_dynamic_size_little_endian.rs | 7 ++++ ...rray_dynamic_element_size_little_endian.rs | 7 ++++ ..._element_size_static_count_1_big_endian.rs | 7 ++++ ...ement_size_static_count_1_little_endian.rs | 7 ++++ ...ic_element_size_static_count_big_endian.rs | 7 ++++ ...element_size_static_count_little_endian.rs | 7 ++++ 16 files changed, 185 insertions(+), 8 deletions(-) diff --git a/pdl-compiler/src/backends/cxx.rs b/pdl-compiler/src/backends/cxx.rs index 434f89c..64f7708 100644 --- a/pdl-compiler/src/backends/cxx.rs +++ b/pdl-compiler/src/backends/cxx.rs @@ -667,12 +667,18 @@ impl<'a> FieldParser<'a> { } (ElementSize::Dynamic, ArraySize::StaticCount(count)) => { + self.append(format!("if ({id}_element_size_ == 0) {{")); + self.append(" return false;".to_string()); + self.append("}".to_string()); self.check_size(&format!("{id}_element_size_ * {count}")); self.append(format!("{id}_ = span.subrange(0, {id}_element_size_ * {count});")); self.append(format!("span.skip({id}_element_size_ * {count});")); } (ElementSize::Dynamic, ArraySize::DynamicCount) => { + self.append(format!("if ({id}_element_size_ == 0) {{")); + self.append(" return false;".to_string()); + self.append("}".to_string()); self.check_size(&format!("{id}_element_size_ * {id}_count_")); self.append(format!("{id}_ = span.subrange(0, {id}_element_size_ * {id}_count_);")); self.append(format!("span.skip({id}_element_size_ * {id}_count_);")); @@ -680,7 +686,9 @@ impl<'a> FieldParser<'a> { (ElementSize::Dynamic, ArraySize::DynamicSize) => { self.check_size(&format!("{id}_size_")); - self.append(format!("if (({id}_size_ % {id}_element_size_) != 0) {{")); + self.append(format!( + "if ({id}_element_size_ == 0 || ({id}_size_ % {id}_element_size_) != 0) {{" + )); self.append(" return false;".to_string()); self.append("}".to_string()); self.append(format!("{id}_ = span.subrange(0, {id}_size_);")); @@ -688,7 +696,9 @@ impl<'a> FieldParser<'a> { } (ElementSize::Dynamic, ArraySize::Unknown) => { - self.append(format!("if ((span.size() % {id}_element_size_) != 0) {{")); + self.append(format!( + "if ({id}_element_size_ == 0 || (span.size() % {id}_element_size_) != 0) {{" + )); self.append(" return false;".to_string()); self.append("}".to_string()); self.append(format!("{id}_ = span;")); @@ -898,7 +908,7 @@ impl<'a> FieldParser<'a> { (ElementSize::Dynamic, ArraySize::DynamicSize) => { self.check_size(&format!("output->{id}_size_")); self.append(format!( - "if ((output->{id}_size_ % output->{id}_element_size_) != 0) {{" + "if (output->{id}_element_size_ == 0 || (output->{id}_size_ % output->{id}_element_size_) != 0) {{" )); self.append(" return false;".to_string()); self.append("}".to_string()); @@ -914,7 +924,7 @@ impl<'a> FieldParser<'a> { } (ElementSize::Dynamic, ArraySize::Unknown) => { - self.append(format!("if ((span.size() % output->{id}_element_size_) != 0) {{")); + self.append(format!("if (output->{id}_element_size_ == 0 || (span.size() % output->{id}_element_size_) != 0) {{")); self.append(" return false;".to_string()); self.append("}".to_string()); self.append(format!( diff --git a/pdl-compiler/src/backends/rust/decoder.rs b/pdl-compiler/src/backends/rust/decoder.rs index 90f6020..ab69897 100644 --- a/pdl-compiler/src/backends/rust/decoder.rs +++ b/pdl-compiler/src/backends/rust/decoder.rs @@ -532,6 +532,17 @@ impl<'a> FieldParser<'a> { }); } (ElementWidth::Dynamic(element_size_field), ArrayShape::Static(count)) => { + // The element size must not be null. + self.tokens.extend(quote! { + if #element_size_field == 0 { + return Err(DecodeError::LengthError { + obj: #packet_name, + wanted: 1, + got: 0, + }); + } + }); + // The element width is known, and the array element // count is known statically. let array_size = if *count == 1 { @@ -569,6 +580,17 @@ impl<'a> FieldParser<'a> { }); } (ElementWidth::Dynamic(element_size_field), ArrayShape::CountField(count_field)) => { + // The element size must not be null. + self.tokens.extend(quote! { + if #element_size_field == 0 { + return Err(DecodeError::LengthError { + obj: #packet_name, + wanted: 1, + got: 0, + }); + } + }); + // The element width is known, and the array element // count is known dynamically by the count field. self.check_size(&span, "e!(#count_field * #element_size_field)); @@ -595,6 +617,17 @@ impl<'a> FieldParser<'a> { } (ElementWidth::Dynamic(element_size_field), ArrayShape::SizeField(_)) | (ElementWidth::Dynamic(element_size_field), ArrayShape::Unknown) => { + // The element size must not be null. + self.tokens.extend(quote! { + if #element_size_field == 0 { + return Err(DecodeError::LengthError { + obj: #packet_name, + wanted: 1, + got: 0, + }); + } + }); + // The element width is known, and the array full size // is known by size field, or unknown (in which case // it is the remaining span length). diff --git a/pdl-compiler/tests/canonical/be_test_vectors.json b/pdl-compiler/tests/canonical/be_test_vectors.json index 3c36195..1f9c6d2 100644 --- a/pdl-compiler/tests/canonical/be_test_vectors.json +++ b/pdl-compiler/tests/canonical/be_test_vectors.json @@ -2154,6 +2154,42 @@ } ] }, + { + "packet": "Packet_Array_Field_VariableElementSize_ConstantSize", + "tests": [ + { + "packed": "000102", + "expected_error": "LengthError" + } + ] + }, + { + "packet": "Packet_Array_Field_VariableElementSize_VariableSize", + "tests": [ + { + "packed": "0100", + "expected_error": "LengthError" + } + ] + }, + { + "packet": "Packet_Array_Field_VariableElementSize_VariableCount", + "tests": [ + { + "packed": "0100", + "expected_error": "LengthError" + } + ] + }, + { + "packet": "Packet_Array_Field_VariableElementSize_UnknownSize", + "tests": [ + { + "packed": "000102", + "expected_error": "LengthError" + } + ] + }, { "packet": "Packet_Optional_Scalar_Field", "tests": [ diff --git a/pdl-compiler/tests/canonical/le_test_vectors.json b/pdl-compiler/tests/canonical/le_test_vectors.json index e6f2e94..c1e1c18 100644 --- a/pdl-compiler/tests/canonical/le_test_vectors.json +++ b/pdl-compiler/tests/canonical/le_test_vectors.json @@ -2217,6 +2217,10 @@ { "packet": "Packet_Array_Field_VariableElementSize_ConstantSize", "tests": [ + { + "packed": "000102", + "expected_error": "LengthError" + }, { "packed": "012a2b2c2d", "unpacked": { @@ -2260,6 +2264,10 @@ { "packet": "Packet_Array_Field_VariableElementSize_VariableSize", "tests": [ + { + "packed": "0100", + "expected_error": "LengthError" + }, { "packed": "01012a2b2c", "unpacked": { @@ -2301,6 +2309,10 @@ { "packet": "Packet_Array_Field_VariableElementSize_VariableCount", "tests": [ + { + "packed": "0100", + "expected_error": "LengthError" + }, { "packed": "03012a2b2c2d", "unpacked": { @@ -2337,6 +2349,10 @@ { "packet": "Packet_Array_Field_VariableElementSize_UnknownSize", "tests": [ + { + "packed": "000102", + "expected_error": "LengthError" + }, { "packed": "012a", "unpacked": { diff --git a/pdl-compiler/tests/generated/cxx/be_backend.h b/pdl-compiler/tests/generated/cxx/be_backend.h index ba8eebb..039e02f 100644 --- a/pdl-compiler/tests/generated/cxx/be_backend.h +++ b/pdl-compiler/tests/generated/cxx/be_backend.h @@ -3799,6 +3799,9 @@ class Packet_Array_Field_VariableElementSize_ConstantSizeView { } uint8_t chunk0 = span.read_be(); array_element_size_ = (chunk0 >> 0) & 0xf; + if (array_element_size_ == 0) { + return false; + } if (span.size() < array_element_size_ * 4) { return false; } @@ -3904,7 +3907,7 @@ class Packet_Array_Field_VariableElementSize_VariableSizeView { if (span.size() < array_size_) { return false; } - if ((array_size_ % array_element_size_) != 0) { + if (array_element_size_ == 0 || (array_size_ % array_element_size_) != 0) { return false; } array_ = span.subrange(0, array_size_); @@ -4020,6 +4023,9 @@ class Packet_Array_Field_VariableElementSize_VariableCountView { array_count_ = (chunk0 >> 0) & 0xf; uint8_t chunk1 = span.read_be(); array_element_size_ = (chunk1 >> 0) & 0xf; + if (array_element_size_ == 0) { + return false; + } if (span.size() < array_element_size_ * array_count_) { return false; } @@ -4123,7 +4129,7 @@ class Packet_Array_Field_VariableElementSize_UnknownSizeView { } uint8_t chunk0 = span.read_be(); array_element_size_ = (chunk0 >> 0) & 0xf; - if ((span.size() % array_element_size_) != 0) { + if (array_element_size_ == 0 || (span.size() % array_element_size_) != 0) { return false; } array_ = span; diff --git a/pdl-compiler/tests/generated/cxx/le_backend.h b/pdl-compiler/tests/generated/cxx/le_backend.h index 41259d5..25738d0 100644 --- a/pdl-compiler/tests/generated/cxx/le_backend.h +++ b/pdl-compiler/tests/generated/cxx/le_backend.h @@ -3799,6 +3799,9 @@ class Packet_Array_Field_VariableElementSize_ConstantSizeView { } uint8_t chunk0 = span.read_le(); array_element_size_ = (chunk0 >> 0) & 0xf; + if (array_element_size_ == 0) { + return false; + } if (span.size() < array_element_size_ * 4) { return false; } @@ -3904,7 +3907,7 @@ class Packet_Array_Field_VariableElementSize_VariableSizeView { if (span.size() < array_size_) { return false; } - if ((array_size_ % array_element_size_) != 0) { + if (array_element_size_ == 0 || (array_size_ % array_element_size_) != 0) { return false; } array_ = span.subrange(0, array_size_); @@ -4020,6 +4023,9 @@ class Packet_Array_Field_VariableElementSize_VariableCountView { array_count_ = (chunk0 >> 0) & 0xf; uint8_t chunk1 = span.read_le(); array_element_size_ = (chunk1 >> 0) & 0xf; + if (array_element_size_ == 0) { + return false; + } if (span.size() < array_element_size_ * array_count_) { return false; } @@ -4123,7 +4129,7 @@ class Packet_Array_Field_VariableElementSize_UnknownSizeView { } uint8_t chunk0 = span.read_le(); array_element_size_ = (chunk0 >> 0) & 0xf; - if ((span.size() % array_element_size_) != 0) { + if (array_element_size_ == 0 || (span.size() % array_element_size_) != 0) { return false; } array_ = span; diff --git a/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_big_endian.rs b/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_big_endian.rs index 98393a5..4ee9404 100644 --- a/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_big_endian.rs +++ b/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_big_endian.rs @@ -127,6 +127,13 @@ impl Packet for Bar { let chunk = buf.get_u8(); let x_element_size = (chunk & 0x1f) as usize; let padding = ((chunk >> 5) & 0x7); + if x_element_size == 0 { + return Err(DecodeError::LengthError { + obj: "Bar", + wanted: 1, + got: 0, + }); + } if buf.remaining() % x_element_size != 0 { return Err(DecodeError::ArraySizeError { array: buf.remaining(), diff --git a/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_dynamic_count_big_endian.rs b/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_dynamic_count_big_endian.rs index 8805264..001f101 100644 --- a/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_dynamic_count_big_endian.rs +++ b/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_dynamic_count_big_endian.rs @@ -123,6 +123,13 @@ impl Packet for Bar { let chunk = buf.get_u8(); let x_count = (chunk & 0xf) as usize; let x_element_size = ((chunk >> 4) & 0xf) as usize; + if x_element_size == 0 { + return Err(DecodeError::LengthError { + obj: "Bar", + wanted: 1, + got: 0, + }); + } if buf.remaining() < x_count * x_element_size { return Err(DecodeError::LengthError { obj: "Bar", diff --git a/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_dynamic_count_little_endian.rs b/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_dynamic_count_little_endian.rs index 8805264..001f101 100644 --- a/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_dynamic_count_little_endian.rs +++ b/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_dynamic_count_little_endian.rs @@ -123,6 +123,13 @@ impl Packet for Bar { let chunk = buf.get_u8(); let x_count = (chunk & 0xf) as usize; let x_element_size = ((chunk >> 4) & 0xf) as usize; + if x_element_size == 0 { + return Err(DecodeError::LengthError { + obj: "Bar", + wanted: 1, + got: 0, + }); + } if buf.remaining() < x_count * x_element_size { return Err(DecodeError::LengthError { obj: "Bar", diff --git a/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_dynamic_size_big_endian.rs b/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_dynamic_size_big_endian.rs index 042bc82..0f08f52 100644 --- a/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_dynamic_size_big_endian.rs +++ b/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_dynamic_size_big_endian.rs @@ -125,6 +125,13 @@ impl Packet for Bar { let chunk = buf.get_u8(); let x_size = (chunk & 0xf) as usize; let x_element_size = ((chunk >> 4) & 0xf) as usize; + if x_element_size == 0 { + return Err(DecodeError::LengthError { + obj: "Bar", + wanted: 1, + got: 0, + }); + } if buf.remaining() < x_size { return Err(DecodeError::LengthError { obj: "Bar", diff --git a/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_dynamic_size_little_endian.rs b/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_dynamic_size_little_endian.rs index 042bc82..0f08f52 100644 --- a/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_dynamic_size_little_endian.rs +++ b/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_dynamic_size_little_endian.rs @@ -125,6 +125,13 @@ impl Packet for Bar { let chunk = buf.get_u8(); let x_size = (chunk & 0xf) as usize; let x_element_size = ((chunk >> 4) & 0xf) as usize; + if x_element_size == 0 { + return Err(DecodeError::LengthError { + obj: "Bar", + wanted: 1, + got: 0, + }); + } if buf.remaining() < x_size { return Err(DecodeError::LengthError { obj: "Bar", diff --git a/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_little_endian.rs b/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_little_endian.rs index 98393a5..4ee9404 100644 --- a/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_little_endian.rs +++ b/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_little_endian.rs @@ -127,6 +127,13 @@ impl Packet for Bar { let chunk = buf.get_u8(); let x_element_size = (chunk & 0x1f) as usize; let padding = ((chunk >> 5) & 0x7); + if x_element_size == 0 { + return Err(DecodeError::LengthError { + obj: "Bar", + wanted: 1, + got: 0, + }); + } if buf.remaining() % x_element_size != 0 { return Err(DecodeError::ArraySizeError { array: buf.remaining(), diff --git a/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_static_count_1_big_endian.rs b/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_static_count_1_big_endian.rs index 160d920..263fa5a 100644 --- a/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_static_count_1_big_endian.rs +++ b/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_static_count_1_big_endian.rs @@ -130,6 +130,13 @@ impl Packet for Bar { let chunk = buf.get_u8(); let x_element_size = (chunk & 0x1f) as usize; let padding = ((chunk >> 5) & 0x7); + if x_element_size == 0 { + return Err(DecodeError::LengthError { + obj: "Bar", + wanted: 1, + got: 0, + }); + } if buf.remaining() < x_element_size { return Err(DecodeError::LengthError { obj: "Bar", diff --git a/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_static_count_1_little_endian.rs b/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_static_count_1_little_endian.rs index 160d920..263fa5a 100644 --- a/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_static_count_1_little_endian.rs +++ b/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_static_count_1_little_endian.rs @@ -130,6 +130,13 @@ impl Packet for Bar { let chunk = buf.get_u8(); let x_element_size = (chunk & 0x1f) as usize; let padding = ((chunk >> 5) & 0x7); + if x_element_size == 0 { + return Err(DecodeError::LengthError { + obj: "Bar", + wanted: 1, + got: 0, + }); + } if buf.remaining() < x_element_size { return Err(DecodeError::LengthError { obj: "Bar", diff --git a/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_static_count_big_endian.rs b/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_static_count_big_endian.rs index ea10d97..7a3cf29 100644 --- a/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_static_count_big_endian.rs +++ b/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_static_count_big_endian.rs @@ -130,6 +130,13 @@ impl Packet for Bar { let chunk = buf.get_u8(); let x_element_size = (chunk & 0x1f) as usize; let padding = ((chunk >> 5) & 0x7); + if x_element_size == 0 { + return Err(DecodeError::LengthError { + obj: "Bar", + wanted: 1, + got: 0, + }); + } if buf.remaining() < 4usize * x_element_size { return Err(DecodeError::LengthError { obj: "Bar", diff --git a/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_static_count_little_endian.rs b/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_static_count_little_endian.rs index ea10d97..7a3cf29 100644 --- a/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_static_count_little_endian.rs +++ b/pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_static_count_little_endian.rs @@ -130,6 +130,13 @@ impl Packet for Bar { let chunk = buf.get_u8(); let x_element_size = (chunk & 0x1f) as usize; let padding = ((chunk >> 5) & 0x7); + if x_element_size == 0 { + return Err(DecodeError::LengthError { + obj: "Bar", + wanted: 1, + got: 0, + }); + } if buf.remaining() < 4usize * x_element_size { return Err(DecodeError::LengthError { obj: "Bar",