diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 4721c6f..3ffd854 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -36,33 +36,6 @@ jobs: - name: cargo fmt --check run: cargo fmt --check - clippy: - runs-on: ubuntu-latest - name: ${{ matrix.toolchain }} / clippy - permissions: - contents: read - checks: write - strategy: - fail-fast: false - matrix: - # Get early warning of new lints which are regularly introduced in beta channels. - toolchain: [stable, beta] - steps: - - uses: actions/checkout@v4 - with: - submodules: true - - name: Install ${{ matrix.toolchain }} - uses: dtolnay/rust-toolchain@master - with: - toolchain: ${{ matrix.toolchain }} - components: clippy - - name: cargo clippy - uses: giraffate/clippy-action@v1 - with: - reporter: 'github-pr-check' - clippy_flags: -- -F clippy::suspicious -F clippy::correctness -F clippy::perf -F clippy::style - github_token: ${{ secrets.GITHUB_TOKEN }} - # Enable once we have a released crate # semver: # runs-on: ubuntu-latest @@ -106,12 +79,16 @@ jobs: submodules: true - name: Install stable uses: dtolnay/rust-toolchain@stable + with: + components: clippy - name: cargo install cargo-hack uses: taiki-e/install-action@cargo-hack # intentionally no target specifier; see https://github.com/jonhoo/rust-ci-conf/pull/4 # --feature-powerset runs for every combination of features - name: cargo hack run: cargo hack --feature-powerset --mutually-exclusive-features=log,defmt check + - name : cargo hack clippy + run: cargo hack --feature-powerset --mutually-exclusive-features=log,defmt clippy -- -Dwarnings deny: # cargo-deny checks licenses, advisories, sources, and bans for diff --git a/Cargo.toml b/Cargo.toml index 2766d8d..2860ae4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,4 +16,21 @@ defmt = [ ] log = [ "dep:log", -] \ No newline at end of file +] + +[lints.rust] +unsafe_code = "forbid" + +[lints.clippy] +correctness = "deny" +expect_used = "deny" +indexing_slicing = "deny" +panic = "deny" +panic_in_result_fn = "deny" +perf = "deny" +suspicious = "deny" +style = "deny" +todo = "deny" +unimplemented = "deny" +unreachable = "deny" +unwrap_used = "deny" diff --git a/src/host.rs b/src/host.rs index 35f9a71..dd430cd 100644 --- a/src/host.rs +++ b/src/host.rs @@ -99,7 +99,8 @@ impl CfuUpdateContent for CfuUpdater { let remainder = total_bytes % chunk_size; // Read and process data in chunks so as to not over-burden memory resources - let mut resp = FwUpdateContentResponse::new(0, CfuUpdateContentResponseStatus::ErrorInvalid); + let mut resp: FwUpdateContentResponse = + FwUpdateContentResponse::new(0, CfuUpdateContentResponseStatus::ErrorInvalid); for i in 0..num_chunks { let mut chunk = [0u8; DEFAULT_DATA_LENGTH]; let address_offset = i * DEFAULT_DATA_LENGTH + base_offset; @@ -120,7 +121,12 @@ impl CfuUpdateContent for CfuUpdater { } _ => { image - .get_bytes_for_chunk(&mut chunk[..remainder], address_offset) + .get_bytes_for_chunk( + chunk + .get_mut(0..remainder) + .ok_or(CfuProtocolError::WriterError(CfuWriterError::Other))?, + address_offset, + ) .await .map_err(|_| CfuProtocolError::WriterError(CfuWriterError::StorageError))?; self.process_last_data_block(writer, chunk, i).await diff --git a/src/protocol_definitions.rs b/src/protocol_definitions.rs index 9182072..6ddd6ec 100644 --- a/src/protocol_definitions.rs +++ b/src/protocol_definitions.rs @@ -169,8 +169,10 @@ impl Default for FwVerComponentInfo { } // Convert to bytes -impl From<&GetFwVersionResponse> for [u8; 60] { - fn from(response: &GetFwVersionResponse) -> Self { +impl TryFrom<&GetFwVersionResponse> for [u8; 60] { + type Error = ConversionError; + + fn try_from(response: &GetFwVersionResponse) -> Result { let mut bytes = [0u8; 60]; // Serialize header @@ -181,17 +183,26 @@ impl From<&GetFwVersionResponse> for [u8; 60] { // Serialize component_info let mut offset = 4; for i in 0..response.header.component_count as usize { - let component = &response.component_info[i]; - bytes[offset] = component.packed_byte; - bytes[offset + 1] = component.component_id; - bytes[offset + 2..offset + 4].copy_from_slice(&component.vendor_specific1.to_le_bytes()); - bytes[offset + 4] = component.fw_version.major; - bytes[offset + 5..offset + 7].copy_from_slice(&component.fw_version.minor.to_le_bytes()); - bytes[offset + 7] = component.fw_version.variant; + let component = &response + .component_info + .get(i) + .ok_or(ConversionError::ByteConversionError)?; + *bytes.get_mut(offset).ok_or(ConversionError::ByteConversionError)? = component.packed_byte; + *bytes.get_mut(offset + 1).ok_or(ConversionError::ByteConversionError)? = component.component_id; + bytes + .get_mut(offset + 2..offset + 4) + .ok_or(ConversionError::ByteConversionError)? + .copy_from_slice(&component.vendor_specific1.to_le_bytes()); + *bytes.get_mut(offset + 4).ok_or(ConversionError::ByteConversionError)? = component.fw_version.major; + bytes + .get_mut(offset + 5..offset + 7) + .ok_or(ConversionError::ByteConversionError)? + .copy_from_slice(&component.fw_version.minor.to_le_bytes()); + *bytes.get_mut(offset + 7).ok_or(ConversionError::ByteConversionError)? = component.fw_version.variant; offset += 8; } - bytes + Ok(bytes) } } @@ -199,14 +210,21 @@ impl From<&GetFwVersionResponse> for [u8; 60] { impl TryFrom<&[u8; 60]> for GetFwVersionResponse { type Error = ConversionError; + #[allow(clippy::indexing_slicing)] // static_check and fixed size array guarantees indexing is safe fn try_from(bytes: &[u8; 60]) -> Result { + const _: () = assert!(MAX_CMPT_COUNT * 8 + 4 <= 60, "Component count exceeds maximum allowed"); + let component_count = bytes[0]; if component_count as usize > MAX_CMPT_COUNT { return Err(ConversionError::ValueOutOfRange); } - let _reserved = u16::from_le_bytes(bytes[1..3].try_into().unwrap()); + let _reserved = u16::from_le_bytes( + bytes[1..3] + .try_into() + .map_err(|_| ConversionError::ByteConversionError)?, + ); let byte3 = match bytes[3] { 0x20 => GetFwVerRespHeaderByte3::NoSpecialFlags, 0x21 => GetFwVerRespHeaderByte3::ExtensionFlagSet, @@ -218,9 +236,17 @@ impl TryFrom<&[u8; 60]> for GetFwVersionResponse { for component in component_info.iter_mut().take(component_count as usize) { component.packed_byte = bytes[offset]; component.component_id = bytes[offset + 1]; - component.vendor_specific1 = u16::from_le_bytes(bytes[offset + 2..offset + 4].try_into().unwrap()); + component.vendor_specific1 = u16::from_le_bytes( + bytes[offset + 2..offset + 4] + .try_into() + .map_err(|_| ConversionError::ByteConversionError)?, + ); component.fw_version.major = bytes[offset + 4]; - component.fw_version.minor = u16::from_le_bytes(bytes[offset + 5..offset + 7].try_into().unwrap()); + component.fw_version.minor = u16::from_le_bytes( + bytes[offset + 5..offset + 7] + .try_into() + .map_err(|_| ConversionError::ByteConversionError)?, + ); component.fw_version.variant = bytes[offset + 7]; offset += 8; } @@ -309,12 +335,24 @@ impl TryFrom<&[u8; 32]> for FwUpdateOffer { let firmware_version = FwVersion { major: bytes[7], - minor: u16::from_le_bytes(bytes[5..7].try_into().unwrap()), + minor: u16::from_le_bytes( + bytes[5..7] + .try_into() + .map_err(|_| ConversionError::ByteConversionError)?, + ), variant: bytes[4], }; - let vendor_specific = u32::from_le_bytes(bytes[8..12].try_into().unwrap()); - let misc_and_protocol_version = u32::from_le_bytes(bytes[12..16].try_into().unwrap()); + let vendor_specific = u32::from_le_bytes( + bytes[8..12] + .try_into() + .map_err(|_| ConversionError::ByteConversionError)?, + ); + let misc_and_protocol_version = u32::from_le_bytes( + bytes[12..16] + .try_into() + .map_err(|_| ConversionError::ByteConversionError)?, + ); Ok(FwUpdateOffer { component_info, @@ -659,8 +697,16 @@ impl TryFrom<&[u8; 60]> for FwUpdateContentCommand { fn try_from(bytes: &[u8; 60]) -> Result { let flags = bytes[0]; let data_length = bytes[1]; - let sequence_num = u16::from_le_bytes(bytes[2..4].try_into().unwrap()); - let firmware_address = u32::from_le_bytes(bytes[4..8].try_into().unwrap()); + let sequence_num = u16::from_le_bytes( + bytes[2..4] + .try_into() + .map_err(|_| ConversionError::ByteConversionError)?, + ); + let firmware_address = u32::from_le_bytes( + bytes[4..8] + .try_into() + .map_err(|_| ConversionError::ByteConversionError)?, + ); let mut data = [0u8; DEFAULT_DATA_LENGTH]; data.copy_from_slice(&bytes[8..]); @@ -1156,7 +1202,7 @@ mod tests { }; // Serialize the fwversion_response_orig to a byte array - let fwversion_response_serialized: [u8; 60] = (&fwversion_response_orig).into(); + let fwversion_response_serialized: [u8; 60] = (&fwversion_response_orig).try_into().unwrap(); // Deserialize the byte array back to a GetFwVersionResponse instance let fwversion_response_deserialized = GetFwVersionResponse::try_from(&fwversion_response_serialized);