Added support for Matter OTA firmware #815

rhilseth · 2025-01-01T19:15:41Z

This PR adds support for detecting Matter OTA firmware (https://docs.nordicsemi.com/bundle/ncs-latest/page/nrf/protocols/matter/overview/dfu.html#matter_ota_image_structure_and_transfer) and also extracting the firmware payload itself.

Example output for the file https://prod.ota-updates.bosch-smarthome.com/v1/updates/BTH-RAD/BTH-RAD_Prod_2.0.9.ota found via the DCL UI at https://webui.dcl.csa-iot.org/models

The test file in tests/inputs has been created manually with a dummy payload.

--------------------------------------------------------------------------------------------------------------------------------------------
DECIMAL                            HEXADECIMAL                        DESCRIPTION
--------------------------------------------------------------------------------------------------------------------------------------------
0                                  0x0                                Matter OTA firmware, total size: 786644 bytes, tlv header size: 68
                                                                      bytes, vendor id: 0x1209, product id: 0x3012, version: 2.00.09,
                                                                      payload size: 786560 bytes, digest type: 1, payload digest:
                                                                      9d4aac8bf87b4ce519318952a3736973e0427d867f3612d776dcd36340fdb574
760368                             0xB9A30                            CRC32 polynomial table, little endian
--------------------------------------------------------------------------------------------------------------------------------------------

devttys0

This looks good to me, aside from the noted sanity checks that should be performed to prevent crashes/exceptions in the event of malformed/corrupted data. Once those are addressed, I'll merge the PR. Thanks!

devttys0 · 2025-01-07T15:51:10Z

src/extractors/matter_ota.rs

+        let total_header_size =
+            MAGIC_SIZE + TOTAL_SIZE_SIZE + HEADER_SIZE_SIZE + ota_header.header_size;
+
+        result.size = Some(ota_header.total_size);


By convention internal extractors should set result.success = true here, so that if the extractor will still return success if it is invoked as part of a dry-run (i.e., output_directory is None).

result.success is now set to true even when there is a dry run

devttys0 · 2025-01-07T15:53:20Z

src/structures/matter_ota.rs

+        // Header starts after the magic, total size and header size fields
+        let header_start = common::size(&ota_structure);
+        let header_end = header_start + header_size;
+        let header_data = &ota_data[header_start..header_end];


There is no guarantee that the values reported in the ota_stucture are sane, thus the calculated header_start / header_end values could cause a crash here. Consider using ota_data.get() and/or sanity checking the header_start and header_end values.

I changed this to using ota_data.get()

devttys0 · 2025-01-07T16:12:27Z

src/structures/matter_ota.rs

+        _ => return Err(StructureError),
+    };
+
+    let field_data = &data[field_offset..];


Although field_offset is a small value, there does not appear to be any guarantee that data is at least field_offset bytes in size. Consider using data.get() or sanity checking the size of data.

I now use data.get() here as well.

devttys0 · 2025-01-07T16:14:20Z

src/structures/matter_ota.rs

+            let structure = &vec![("string_length", field_width_type)];
+            let result = common::parse(field_data, structure, "little")?;
+            let string_length = result["string_length"] as usize;
+            let string_data = &field_data[common::size(structure)..];


Consider using field_data.get() or sanity checking the size of field_data.

Using field_data.get() here.

devttys0 · 2025-01-07T16:15:59Z

src/structures/matter_ota.rs

+            let result = common::parse(field_data, structure, "little")?;
+            let string_length = result["string_length"] as usize;
+            let string_data = &field_data[common::size(structure)..];
+            let string = get_cstring(&string_data[..string_length]);


Consider using string_data.get() or sanity checking the size of string_data. Alternatively, you can simply pass string_data directly to get_cstring(), as it will terminate at the first null byte or the end of data, whichever comes first.

The TLV encoded strings aren't null-terminated in the headers I've seen. But I changed to using string_data.get() here as well

devttys0 · 2025-01-07T16:16:18Z

src/structures/matter_ota.rs

+            let structure = &vec![("octet_string_length", field_width_type)];
+            let result = common::parse(field_data, structure, "little")?;
+            let octet_string_length = result["octet_string_length"] as usize;
+            let octet_string_data = &field_data[common::size(structure)..];


Consider using field_data.get() or sanity checking the size of field_data.

Using field_data.get() now

devttys0 · 2025-01-07T16:16:44Z

src/structures/matter_ota.rs

+            Ok((
+                Element {
+                    tag,
+                    value: Value::OctetString(octet_string_data[..octet_string_length].to_vec()),


Consider using octet_string_data.get() or sanity checking the size of octet_string_data.

Using octet_string_data.get() here

devttys0 · 2025-01-07T16:18:09Z

src/structures/matter_ota.rs

+    ];
+    let mut offset = 0;
+    let mut header = HashMap::new();
+    while offset < data.len() {


Consider sanity checking the offset value by using while common::is_offset_safe() here to prevent infinite loops, wrap-arounds, and out-of-bounds access.

I changed to using common::is_offset_safe()

Thanks for your review comments!

Added support for Matter OTA firmware

e9d690f

devttys0 requested changes Jan 7, 2025

View reviewed changes

More sanity checks on matter_ota data

1d01a5f

rhilseth requested a review from devttys0 January 7, 2025 17:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for Matter OTA firmware #815

Added support for Matter OTA firmware #815

rhilseth commented Jan 1, 2025

devttys0 left a comment

devttys0 Jan 7, 2025

rhilseth Jan 7, 2025

devttys0 Jan 7, 2025

rhilseth Jan 7, 2025

devttys0 Jan 7, 2025

rhilseth Jan 7, 2025

devttys0 Jan 7, 2025

rhilseth Jan 7, 2025

devttys0 Jan 7, 2025

rhilseth Jan 7, 2025

devttys0 Jan 7, 2025

rhilseth Jan 7, 2025

devttys0 Jan 7, 2025

rhilseth Jan 7, 2025

devttys0 Jan 7, 2025

rhilseth Jan 7, 2025

Added support for Matter OTA firmware #815

Are you sure you want to change the base?

Added support for Matter OTA firmware #815

Conversation

rhilseth commented Jan 1, 2025

devttys0 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment