Skip to content

Commit

Permalink
apple-codesign: add padding 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 a bug was filed.

So this commit teaches the segment writing code to pad the space
between segments with zeroes so this case is handled.

Part of #616.
  • Loading branch information
indygreg committed Aug 4, 2022
1 parent bce3336 commit 360fdae
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 2 deletions.
2 changes: 2 additions & 0 deletions apple-codesign/CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

* minicbor crate upgraded from version 0.15. This created API differences in
remote signing code.
* Fixed a bug where we hit an assertion when adding code signatures to Mach-O
binaries having padding between segments. (#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 @@ -164,6 +164,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
30 changes: 28 additions & 2 deletions apple-codesign/src/macho_signing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,15 +163,41 @@ fn create_macho_with_signature(

// Write out segments, updating the __LINKEDIT segment when we encounter it.
for segment in 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) {
// Most Mach-O are observed to have no padding the file-level segment offsets: usually
// padding is done within the segment span. But it is valid for there to be padding
// between segments. If this occurs, just pad with null bytes.
Ordering::Less => {
let padding = b"\0".repeat((segment.fileoff - cursor.position()) as usize);
let name = segment.name().unwrap_or("<unknown>");
warn!("padding {} bytes before segment {}", padding.len(), name);
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 360fdae

Please sign in to comment.