diff --git a/apple-codesign/CHANGELOG.rst b/apple-codesign/CHANGELOG.rst index 5c16f461f..be033466c 100644 --- a/apple-codesign/CHANGELOG.rst +++ b/apple-codesign/CHANGELOG.rst @@ -43,6 +43,8 @@ to custom Mach-O functionality. Most code interfacing with a Mach-O file now uses these types. The ``AppleSignable`` trait has been deleted as it is no longer needed since we have the dedicated ``MachOBinary`` type. +* Fixed a bug where we hit an assertion when adding code signatures to Mach-O + binaries having padding between segments. (#588 and #616) 0.16.0 ====== diff --git a/apple-codesign/src/error.rs b/apple-codesign/src/error.rs index 6054d9c5e..61de69264 100644 --- a/apple-codesign/src/error.rs +++ b/apple-codesign/src/error.rs @@ -167,6 +167,9 @@ pub enum AppleCodesignError { #[error("insufficient room to write code signature load command")] LoadCommandNoRoom, + #[error("error writing Mach-O: {0}")] + MachOWrite(String), + #[error("no identifier string provided")] NoIdentifier, diff --git a/apple-codesign/src/macho_signing.rs b/apple-codesign/src/macho_signing.rs index d75df1a60..7b3d0f4e5 100644 --- a/apple-codesign/src/macho_signing.rs +++ b/apple-codesign/src/macho_signing.rs @@ -30,7 +30,7 @@ use { }, parse_magic_and_ctx, }, - log::{info, warn}, + log::{debug, info, warn}, scroll::{ctx::SizeWith, IOwrite}, std::{borrow::Cow, cmp::Ordering, collections::HashMap, io::Write, path::Path}, tugger_apple::create_universal_macho, @@ -162,8 +162,6 @@ fn create_macho_with_signature( // Write out segments, updating the __LINKEDIT segment when we encounter it. for segment in macho.macho.segments.iter() { - assert!(segment.fileoff == 0 || segment.fileoff == cursor.position()); - // The initial __PAGEZERO segment contains no data (it is the magic and load // commands) and overlaps with the __TEXT segment, which has .fileoff =0, so // we ignore it. @@ -171,6 +169,36 @@ fn create_macho_with_signature( continue; } + match cursor.position().cmp(&segment.fileoff) { + // Mach-O segments may have padding between them. In this case, copy these + // bytes (presumably NULLs but that isn't guaranteed) to the output. + Ordering::Less => { + let padding = &macho.data[cursor.position() as usize..segment.fileoff as usize]; + debug!( + "copying {} bytes outside segment boundaries before segment {}", + padding.len(), + segment.name().unwrap_or("") + ); + cursor.write_all(&padding)?; + } + // The __TEXT segment usually has .fileoff = 0, which has it overlapping with + // already written data. Allow this special case through. + Ordering::Greater if segment.fileoff == 0 => {} + + // The writer has overran into this segment. That means we screwed up on a + // previous loop iteration. + Ordering::Greater => { + return Err(AppleCodesignError::MachOWrite(format!( + "Mach-O segment corruption: cursor at 0x{:x} but segment begins at 0x{:x} (please report this bug)", + cursor.position(), + segment.fileoff + ))); + } + Ordering::Equal => {} + } + + assert!(segment.fileoff == 0 || segment.fileoff == cursor.position()); + match segment.name() { Ok(SEG_LINKEDIT) => { cursor.write_all(