Skip to content

Avoid unsafe when decoding #303

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

Open
JohnAZoidberg wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Avoid unsafe when decoding #303

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
21 changes: 21 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions framework_lib/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ no-std-compat = { version = "0.4.1", features = [ "alloc" ] }
hidapi = { version = "2.6.3", features = [ "windows-native" ], optional = true }
rusb = { version = "0.9.4", optional = true }
guid-create = { version = "0.5.0", default-features = false }
zerocopy = { version = "0.8", default-features = false, features = ["derive"] }

[target.'cfg(target_os = "uefi")'.dependencies]
uefi = { version = "0.36.1", features = ["alloc", "global_allocator", "panic_handler", "logger"] }
Expand Down
38 changes: 30 additions & 8 deletions framework_lib/src/capsule.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
use std::prelude::v1::*;

use core::prelude::rust_2021::derive;
use guid_create::CGuid;
use guid_create::{CGuid, GUID};
#[cfg(not(feature = "uefi"))]
use std::fs::File;
#[cfg(not(feature = "uefi"))]
Expand Down Expand Up @@ -112,10 +112,22 @@ fn print_capsule_flags(flags: u32) {
}
}

fn read_capsule_header(data: &[u8]) -> EfiCapsuleHeader {
EfiCapsuleHeader {
capsule_guid: CGuid::from(GUID::build_from_components(
u32::from_le_bytes(data[0..4].try_into().unwrap()),
u16::from_le_bytes(data[4..6].try_into().unwrap()),
u16::from_le_bytes(data[6..8].try_into().unwrap()),
data[8..16].try_into().unwrap(),
)),
header_size: u32::from_le_bytes(data[16..20].try_into().unwrap()),
flags: u32::from_le_bytes(data[20..24].try_into().unwrap()),
capsule_image_size: u32::from_le_bytes(data[24..28].try_into().unwrap()),
}
}

pub fn parse_capsule_header(data: &[u8]) -> Option<EfiCapsuleHeader> {
let header_len = std::mem::size_of::<EfiCapsuleHeader>();
let header: EfiCapsuleHeader =
unsafe { std::ptr::read(data[0..header_len].as_ptr() as *const _) };
let header = read_capsule_header(data);
if header.is_valid(data) {
Some(header)
} else {
Expand All @@ -141,10 +153,20 @@ pub fn print_capsule_header(header: &EfiCapsuleHeader) {
}

pub fn parse_ux_header(data: &[u8]) -> DisplayCapsule {
let header_len = std::mem::size_of::<DisplayCapsule>();
let header: DisplayCapsule =
unsafe { std::ptr::read(data[0..header_len].as_ptr() as *const _) };
header
let capsule_header = read_capsule_header(data);
let p = &data[std::mem::size_of::<EfiCapsuleHeader>()..];
DisplayCapsule {
capsule_header,
image_payload: DisplayPayload {
version: p[0],
checksum: p[1],
image_type: p[2],
reserved: p[3],
mode: u32::from_le_bytes(p[4..8].try_into().unwrap()),
offset_x: u32::from_le_bytes(p[8..12].try_into().unwrap()),
offset_y: u32::from_le_bytes(p[12..16].try_into().unwrap()),
},
}
}
pub fn print_ux_header(header: &DisplayCapsule) {
let header_len = std::mem::size_of::<DisplayCapsule>();
Expand Down
24 changes: 12 additions & 12 deletions framework_lib/src/ccgx/binary.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ use alloc::vec::Vec;
use core::prelude::rust_2021::derive;

use crate::ccgx::{AppVersion, BaseVersion};
use zerocopy::byteorder::little_endian::{U16, U32};
use zerocopy::{FromBytes, KnownLayout};

use super::*;

Expand All @@ -45,12 +47,12 @@ const SMALL_ROW: usize = 0x80;
const LARGE_ROW: usize = 0x100;

#[repr(C, packed)]
#[derive(Debug, Copy, Clone)]
#[derive(FromBytes, KnownLayout, Debug, Copy, Clone)]
struct VersionInfo {
base_version: u32,
app_version: u32,
silicon_id: u16,
silicon_family: u16,
base_version: U32,
app_version: U32,
silicon_id: U16,
silicon_family: U16,
}

pub const CCG5_PD_LEN: usize = 0x20_000;
Expand Down Expand Up @@ -147,15 +149,13 @@ fn read_version(
trace!("First row of firmware: {:X?}", data);
let data = &data[FW_VERSION_OFFSET..];

let version_len = std::mem::size_of::<VersionInfo>();
let version_info: VersionInfo =
unsafe { std::ptr::read(data[0..version_len].as_ptr() as *const _) };
let (version_info, _) = VersionInfo::read_from_prefix(data).ok()?;

let base_version = BaseVersion::from(version_info.base_version);
let app_version = AppVersion::from(version_info.app_version);
let base_version = BaseVersion::from(version_info.base_version.get());
let app_version = AppVersion::from(version_info.app_version.get());

let fw_silicon_id = version_info.silicon_id;
let fw_silicon_family = version_info.silicon_family;
let fw_silicon_id = version_info.silicon_id.get();
let fw_silicon_family = version_info.silicon_family.get();

Some(PdFirmware {
silicon_id: fw_silicon_id,
Expand Down
52 changes: 27 additions & 25 deletions framework_lib/src/ccgx/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ use alloc::vec::Vec;
use core::prelude::rust_2021::derive;
use num_derive::FromPrimitive;
use std::fmt;
use zerocopy::byteorder::little_endian::{U16, U32};
use zerocopy::{FromBytes, KnownLayout};

use crate::chromium_ec::{CrosEc, EcResult};
use crate::smbios;
Expand All @@ -33,18 +35,18 @@ const METADATA_MAGIC: u16 = u16::from_le_bytes([b'Y', b'C']); // CY (Cypress)
const CCG8_METADATA_MAGIC: u16 = u16::from_le_bytes([b'F', b'I']); // IF (Infineon)

#[repr(C, packed)]
#[derive(Debug, Copy, Clone)]
#[derive(FromBytes, KnownLayout, Debug, Copy, Clone)]
struct CyAcdMetadata {
/// Offset 00: Single Byte FW Checksum
_fw_checksum: u8,
/// Offset 01: FW Entry Address
_fw_entry: u32,
/// Offset 05: Last Flash row of Bootloader or previous firmware
boot_last_row: u16,
boot_last_row: U16,
/// Offset 07: Reserved
_reserved1: [u8; 2],
/// Offset 09: Size of Firmware
fw_size: u32,
fw_size: U32,
/// Offset 0D: Reserved
_reserved2: [u8; 3],
/// Offset 10: Creator specific field
Expand All @@ -56,7 +58,7 @@ struct CyAcdMetadata {
/// Offset 14: Creator specific field
_boot_app_id: u16,
/// Offset 16: Metadata Valid field. Valid if contains "CY"
metadata_valid: u16,
metadata_valid: U16,
/// Offset 18: Creator specific field
_fw_version: u32,
/// Offset 1C: Boot sequence number field. Boot-loader will load the valid
Expand All @@ -67,12 +69,12 @@ struct CyAcdMetadata {

// TODO: Would be nice to check the checksums
#[repr(C, packed)]
#[derive(Debug, Copy, Clone)]
#[derive(FromBytes, KnownLayout, Debug, Copy, Clone)]
struct CyAcd2Metadata {
/// Offset 00: App Firmware Start
fw_start: u32,
fw_start: U32,
/// Offset 04: App Firmware Size
fw_size: u32,
fw_size: U32,
/// Offset 08: Boot wait time
_boot_app_id: u16,
/// Offset 0A: Last Flash row of Bootloader or previous firmware
Expand All @@ -89,9 +91,9 @@ struct CyAcd2Metadata {
/// Offset 18: Reserved
_reserved_1: [u32; 15],
/// Offset 54: Version of the metadata structure
metadata_version: u16,
metadata_version: U16,
/// Offset 56: Metadata Valid field. Valid if contains ASCII "IF"
metadata_valid: u16,
metadata_valid: U16,
/// Offset 58: App Fw CRC32 checksum
_fw_crc32: u32,
/// Offset 5C: Reserved
Expand Down Expand Up @@ -267,12 +269,13 @@ pub fn get_pd_controller_versions(ec: &CrosEc) -> EcResult<PdVersions> {

fn parse_metadata_ccg3(buffer: &[u8]) -> Option<(u32, u32)> {
let buffer = &buffer[CCG3_METADATA_OFFSET..];
let metadata_len = std::mem::size_of::<CyAcdMetadata>();
let metadata: CyAcdMetadata =
unsafe { std::ptr::read(buffer[0..metadata_len].as_ptr() as *const _) };
let (metadata, _) = CyAcdMetadata::read_from_prefix(buffer).ok()?;
trace!("Metadata: {:X?}", metadata);
if metadata.metadata_valid == METADATA_MAGIC {
Some((1 + metadata.boot_last_row as u32, metadata.fw_size))
if metadata.metadata_valid.get() == METADATA_MAGIC {
Some((
1 + metadata.boot_last_row.get() as u32,
metadata.fw_size.get(),
))
} else {
None
}
Expand All @@ -281,26 +284,25 @@ fn parse_metadata_ccg3(buffer: &[u8]) -> Option<(u32, u32)> {
//fn parse_metadata(buffer: &[u8; 256]) -> Option<(u32, u32)> {
fn parse_metadata_cyacd(buffer: &[u8]) -> Option<(u32, u32)> {
let buffer = &buffer[METADATA_OFFSET..];
let metadata_len = std::mem::size_of::<CyAcdMetadata>();
let metadata: CyAcdMetadata =
unsafe { std::ptr::read(buffer[0..metadata_len].as_ptr() as *const _) };
let (metadata, _) = CyAcdMetadata::read_from_prefix(buffer).ok()?;
trace!("Metadata: {:X?}", metadata);
if metadata.metadata_valid == METADATA_MAGIC {
Some((1 + metadata.boot_last_row as u32, metadata.fw_size))
if metadata.metadata_valid.get() == METADATA_MAGIC {
Some((
1 + metadata.boot_last_row.get() as u32,
metadata.fw_size.get(),
))
} else {
None
}
}

fn parse_metadata_cyacd2(buffer: &[u8]) -> Option<(u32, u32)> {
let buffer = &buffer[CCG8_METADATA_OFFSET..];
let metadata_len = std::mem::size_of::<CyAcd2Metadata>();
let metadata: CyAcd2Metadata =
unsafe { std::ptr::read(buffer[0..metadata_len].as_ptr() as *const _) };
let (metadata, _) = CyAcd2Metadata::read_from_prefix(buffer).ok()?;
trace!("Metadata: {:X?}", metadata);
if metadata.metadata_valid == CCG8_METADATA_MAGIC {
if metadata.metadata_version == 1 {
Some((metadata.fw_start, metadata.fw_size))
if metadata.metadata_valid.get() == CCG8_METADATA_MAGIC {
if metadata.metadata_version.get() == 1 {
Some((metadata.fw_start.get(), metadata.fw_size.get()))
} else {
println!("Unknown CCG8 metadata version");
None
Expand Down
71 changes: 35 additions & 36 deletions framework_lib/src/ec_binary.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,21 @@ const EC_RW_VER_OFFSET_ZEPHYR: usize = 0x40140;
pub const EC_LEN: usize = 0x8_0000;

use regex;
use zerocopy::byteorder::little_endian::U32;
use zerocopy::{FromBytes, KnownLayout};

#[cfg(feature = "uefi")]
use core::prelude::rust_2021::derive;

// Defined in EC code as `struct image_data` in include/cros_version.h
#[derive(Clone, Copy, Debug)]
#[derive(FromBytes, KnownLayout, Clone, Copy, Debug)]
#[repr(C, packed)]
struct _ImageVersionData {
cookie1: u32,
cookie1: U32,
version: [u8; 32],
size: u32,
rollback_version: u32,
cookie2: u32,
size: U32,
rollback_version: U32,
cookie2: U32,
}
/// Version Information about an EC FW binary
#[derive(Debug, PartialEq)]
Expand Down Expand Up @@ -79,8 +81,8 @@ fn parse_ec_version(data: &_ImageVersionData) -> Option<ImageVersionData> {
.trim_end_matches(char::from(0));
Some(ImageVersionData {
version: version.to_string(),
size: data.size,
rollback_version: data.rollback_version,
size: data.size.get(),
rollback_version: data.rollback_version.get(),
details: parse_ec_version_str(version)?,
})
}
Expand Down Expand Up @@ -151,18 +153,17 @@ pub fn read_ec_version(data: &[u8], ro: bool) -> Option<ImageVersionData> {
} else {
EC_RW_VER_OFFSET
};
if data.len() < offset + core::mem::size_of::<_ImageVersionData>() {
return None;
}
let v: _ImageVersionData = unsafe { std::ptr::read(data[offset..].as_ptr() as *const _) };
if v.cookie1 != CROS_EC_IMAGE_DATA_COOKIE1 {
debug!("Failed to find legacy Cookie 1. Found: {:X?}", {
v.cookie1
});
} else if v.cookie2 != CROS_EC_IMAGE_DATA_COOKIE2 {
debug!("Failed to find legacy Cookie 2. Found: {:X?}", {
v.cookie2
});
let (v, _) = _ImageVersionData::read_from_prefix(data.get(offset..)?).ok()?;
if v.cookie1.get() != CROS_EC_IMAGE_DATA_COOKIE1 {
debug!(
"Failed to find legacy Cookie 1. Found: {:X?}",
v.cookie1.get()
);
} else if v.cookie2.get() != CROS_EC_IMAGE_DATA_COOKIE2 {
debug!(
"Failed to find legacy Cookie 2. Found: {:X?}",
v.cookie2.get()
);
} else {
return parse_ec_version(&v);
}
Expand All @@ -173,19 +174,17 @@ pub fn read_ec_version(data: &[u8], ro: bool) -> Option<ImageVersionData> {
} else {
EC_RW_VER_OFFSET_ZEPHYR
};
if data.len() < offset_zephyr + core::mem::size_of::<_ImageVersionData>() {
return None;
}
let v: _ImageVersionData =
unsafe { std::ptr::read(data[offset_zephyr..].as_ptr() as *const _) };
if v.cookie1 != CROS_EC_IMAGE_DATA_COOKIE1 {
debug!("Failed to find Zephyr Cookie 1. Found: {:X?}", {
v.cookie1
});
} else if v.cookie2 != CROS_EC_IMAGE_DATA_COOKIE2 {
debug!("Failed to find Zephyr Cookie 2. Found: {:X?}", {
v.cookie2
});
let (v, _) = _ImageVersionData::read_from_prefix(data.get(offset_zephyr..)?).ok()?;
if v.cookie1.get() != CROS_EC_IMAGE_DATA_COOKIE1 {
debug!(
"Failed to find Zephyr Cookie 1. Found: {:X?}",
v.cookie1.get()
);
} else if v.cookie2.get() != CROS_EC_IMAGE_DATA_COOKIE2 {
debug!(
"Failed to find Zephyr Cookie 2. Found: {:X?}",
v.cookie2.get()
);
} else {
return parse_ec_version(&v);
}
Expand All @@ -204,11 +203,11 @@ mod tests {
fn can_parse() {
let ver_chars: &[u8] = b"hx30_v0.0.1-7a61a89\0\0\0\0\0\0\0\0\0\0\0\0\0";
let data = _ImageVersionData {
cookie1: CROS_EC_IMAGE_DATA_COOKIE1,
cookie1: U32::new(CROS_EC_IMAGE_DATA_COOKIE1),
version: ver_chars.try_into().unwrap(),
size: 2868,
rollback_version: 0,
cookie2: CROS_EC_IMAGE_DATA_COOKIE1,
size: U32::new(2868),
rollback_version: U32::new(0),
cookie2: U32::new(CROS_EC_IMAGE_DATA_COOKIE1),
};
debug_assert_eq!(
parse_ec_version(&data),
Expand Down
Loading