Skip to content

Missing checks and test cases for invalid element size field #181

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
hchataing merged 1 commit into from
Jul 25, 2026
Merged

Missing checks and test cases for invalid element size field #181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 14 additions & 4 deletions pdl-compiler/src/backends/cxx.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -667,28 +667,38 @@ 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_);"));
}

(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_);"));
self.append(format!("span.skip({id}_size_);"));
}

(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;"));
Expand Down Expand Up @@ -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());
Expand All @@ -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!(
Expand Down
33 changes: 33 additions & 0 deletions pdl-compiler/src/backends/rust/decoder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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, &quote!(#count_field * #element_size_field));
Expand All @@ -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).
Expand Down
36 changes: 36 additions & 0 deletions pdl-compiler/tests/canonical/be_test_vectors.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down
16 changes: 16 additions & 0 deletions pdl-compiler/tests/canonical/le_test_vectors.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2217,6 +2217,10 @@
{
"packet": "Packet_Array_Field_VariableElementSize_ConstantSize",
"tests": [
{
"packed": "000102",
"expected_error": "LengthError"
},
{
"packed": "012a2b2c2d",
"unpacked": {
Expand Down Expand Up @@ -2260,6 +2264,10 @@
{
"packet": "Packet_Array_Field_VariableElementSize_VariableSize",
"tests": [
{
"packed": "0100",
"expected_error": "LengthError"
},
{
"packed": "01012a2b2c",
"unpacked": {
Expand Down Expand Up @@ -2301,6 +2309,10 @@
{
"packet": "Packet_Array_Field_VariableElementSize_VariableCount",
"tests": [
{
"packed": "0100",
"expected_error": "LengthError"
},
{
"packed": "03012a2b2c2d",
"unpacked": {
Expand Down Expand Up @@ -2337,6 +2349,10 @@
{
"packet": "Packet_Array_Field_VariableElementSize_UnknownSize",
"tests": [
{
"packed": "000102",
"expected_error": "LengthError"
},
{
"packed": "012a",
"unpacked": {
Expand Down
10 changes: 8 additions & 2 deletions pdl-compiler/tests/generated/cxx/be_backend.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3799,6 +3799,9 @@ class Packet_Array_Field_VariableElementSize_ConstantSizeView {
}
uint8_t chunk0 = span.read_be<uint8_t, 1>();
array_element_size_ = (chunk0 >> 0) & 0xf;
if (array_element_size_ == 0) {
return false;
}
if (span.size() < array_element_size_ * 4) {
return false;
}
Expand Down Expand Up @@ -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_);
Expand Down Expand Up @@ -4020,6 +4023,9 @@ class Packet_Array_Field_VariableElementSize_VariableCountView {
array_count_ = (chunk0 >> 0) & 0xf;
uint8_t chunk1 = span.read_be<uint8_t, 1>();
array_element_size_ = (chunk1 >> 0) & 0xf;
if (array_element_size_ == 0) {
return false;
}
if (span.size() < array_element_size_ * array_count_) {
return false;
}
Expand Down Expand Up @@ -4123,7 +4129,7 @@ class Packet_Array_Field_VariableElementSize_UnknownSizeView {
}
uint8_t chunk0 = span.read_be<uint8_t, 1>();
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;
Expand Down
10 changes: 8 additions & 2 deletions pdl-compiler/tests/generated/cxx/le_backend.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3799,6 +3799,9 @@ class Packet_Array_Field_VariableElementSize_ConstantSizeView {
}
uint8_t chunk0 = span.read_le<uint8_t, 1>();
array_element_size_ = (chunk0 >> 0) & 0xf;
if (array_element_size_ == 0) {
return false;
}
if (span.size() < array_element_size_ * 4) {
return false;
}
Expand Down Expand Up @@ -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_);
Expand Down Expand Up @@ -4020,6 +4023,9 @@ class Packet_Array_Field_VariableElementSize_VariableCountView {
array_count_ = (chunk0 >> 0) & 0xf;
uint8_t chunk1 = span.read_le<uint8_t, 1>();
array_element_size_ = (chunk1 >> 0) & 0xf;
if (array_element_size_ == 0) {
return false;
}
if (span.size() < array_element_size_ * array_count_) {
return false;
}
Expand Down Expand Up @@ -4123,7 +4129,7 @@ class Packet_Array_Field_VariableElementSize_UnknownSizeView {
}
uint8_t chunk0 = span.read_le<uint8_t, 1>();
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;
Expand Down
7 changes: 7 additions & 0 deletions pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_big_endian.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
7 changes: 7 additions & 0 deletions ...r/tests/generated/rust/packet_decl_array_dynamic_element_size_dynamic_count_big_endian.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
7 changes: 7 additions & 0 deletions ...ests/generated/rust/packet_decl_array_dynamic_element_size_dynamic_count_little_endian.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
7 changes: 7 additions & 0 deletions ...er/tests/generated/rust/packet_decl_array_dynamic_element_size_dynamic_size_big_endian.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
7 changes: 7 additions & 0 deletions ...tests/generated/rust/packet_decl_array_dynamic_element_size_dynamic_size_little_endian.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
7 changes: 7 additions & 0 deletions pdl-compiler/tests/generated/rust/packet_decl_array_dynamic_element_size_little_endian.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
7 changes: 7 additions & 0 deletions .../tests/generated/rust/packet_decl_array_dynamic_element_size_static_count_1_big_endian.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
7 changes: 7 additions & 0 deletions ...sts/generated/rust/packet_decl_array_dynamic_element_size_static_count_1_little_endian.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading
Loading