Skip to content

Commit

Permalink
apple-codesign: copy data between Mach-O segments
Browse files Browse the repository at this point in the history
Before, Mach-O segments that had padding between segments crashed
due to an assertion. Having padding between Mach-O segments is
allowed - albeit not observed until recently.

So this commit teaches the segment writing code to preserve the space
between segments when writing out a Mach-O.

We still fail to handle these gaps when computing code signatures.
This will be addressed in a separate commit.

Part of #588 and #616.
  • Loading branch information
indygreg committed Aug 6, 2022
1 parent ea1bc5f commit 15a7613
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 3 deletions.
2 changes: 2 additions & 0 deletions apple-codesign/CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
======
Expand Down
3 changes: 3 additions & 0 deletions apple-codesign/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Expand Down
34 changes: 31 additions & 3 deletions apple-codesign/src/macho_signing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -162,15 +162,43 @@ 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.
if matches!(segment.name(), Ok(SEG_PAGEZERO)) {
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("<unknown>")
);
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(
Expand Down

0 comments on commit 15a7613

Please sign in to comment.