Skip to content
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.

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

Artificial MachO binary processed by llvm-objcopy/strip is not accepted by codesign #117723

Closed
RIscRIpt opened this issue Nov 26, 2024 · 2 comments · Fixed by #117726
Closed

Artificial MachO binary processed by llvm-objcopy/strip is not accepted by codesign #117723

RIscRIpt opened this issue Nov 26, 2024 · 2 comments · Fixed by #117726

Comments

@RIscRIpt
Copy link
Member

Symptom

Artificial MachO binary processed by llvm-objcopy/strip is not accepted by codesign.

codesign fails with error:

main executable failed strict validation

Investigation

According to Darling/libsecurity_utilities/lib/macho++.cpp

libsecurity_utilities/lib/macho++.cpp|238 col 3| mSuspicious = true;
libsecurity_utilities/lib/macho++.cpp|617 col 7| mSuspicious = true;
libsecurity_utilities/lib/macho++.cpp|629 col 7| mSuspicious = true;
libsecurity_utilities/lib/macho++.cpp|635 col 8| mSuspicious = true;
libsecurity_utilities/lib/macho++.cpp|643 col 6| mSuspicious = true;
libsecurity_utilities/lib/macho++.cpp|654 col 5| mSuspicious = true;

According to MachO::validateStructure

  1. The sum of the last LoadCommand and its size must equal to the total size of "fat" component,
    which sometimes corresponds to the end of file.

According to Universal::Universal

  1. Alignment of "fat" components must be less than 2^30, and
  2. The gap size between "fat" components must be less than its alignment.
  3. The gap must be filled with zero bytes.
  4. No junk shall be past end of file or "fat" component.

Relevant requirement

From the above there should be no data in MachO past contents of a load command with the largest offset+size.

Problem description

llvm-objcopy does not update indirectsymoff field if the resulting IndirectSymTable (after stripping) is empty [1]. As a result indirectsymoff keeps old value, which is garbage, and can point past offset+size of the last load command. As a result totalSize() returns size that is larger than the actual stripped MachO [2].

This clearly violates one of the checks mentioned above (there are zero-bytes filled past offset+size of the last load command.

@RIscRIpt RIscRIpt added bug Indicates an unexpected problem or unintended behavior platform:macos tools:llvm-objcopy/strip labels Nov 26, 2024
@RIscRIpt RIscRIpt self-assigned this Nov 26, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 26, 2024

@llvm/issue-subscribers-bug

Author: Richard Dzenis (RIscRIpt)

## Symptom

Artificial MachO binary processed by llvm-objcopy/strip is not accepted by codesign.

codesign fails with error:

> main executable failed strict validation

Investigation

According to Darling/libsecurity_utilities/lib/macho++.cpp

libsecurity_utilities/lib/macho++.cpp|238 col 3| mSuspicious = true;
libsecurity_utilities/lib/macho++.cpp|617 col 7| mSuspicious = true;
libsecurity_utilities/lib/macho++.cpp|629 col 7| mSuspicious = true;
libsecurity_utilities/lib/macho++.cpp|635 col 8| mSuspicious = true;
libsecurity_utilities/lib/macho++.cpp|643 col 6| mSuspicious = true;
libsecurity_utilities/lib/macho++.cpp|654 col 5| mSuspicious = true;

According to MachO::validateStructure

  1. The sum of the last LoadCommand and its size must equal to the total size of "fat" component,
    which sometimes corresponds to the end of file.

According to Universal::Universal

  1. Alignment of "fat" components must be less than 2^30, and
  2. The gap size between "fat" components must be less than its alignment.
  3. The gap must be filled with zero bytes.
  4. No junk shall be past end of file or "fat" component.

Relevant requirement

From the above there should be no data in MachO past contents of a load command with the largest offset+size.

Problem description

llvm-objcopy does not update indirectsymoff field if the resulting IndirectSymTable (after stripping) is empty [1]. As a result indirectsymoff keeps old value, which is garbage, and can point past offset+size of the last load command. As a result totalSize() returns size that is larger than the actual stripped MachO [2].

This clearly violates one of the checks mentioned above (there are zero-bytes filled past offset+size of the last load command.

@llvmbot
Copy link
Member

llvmbot commented Nov 26, 2024

@llvm/issue-subscribers-tools-llvm-objcopy-strip

Author: Richard Dzenis (RIscRIpt)

## Symptom

Artificial MachO binary processed by llvm-objcopy/strip is not accepted by codesign.

codesign fails with error:

> main executable failed strict validation

Investigation

According to Darling/libsecurity_utilities/lib/macho++.cpp

libsecurity_utilities/lib/macho++.cpp|238 col 3| mSuspicious = true;
libsecurity_utilities/lib/macho++.cpp|617 col 7| mSuspicious = true;
libsecurity_utilities/lib/macho++.cpp|629 col 7| mSuspicious = true;
libsecurity_utilities/lib/macho++.cpp|635 col 8| mSuspicious = true;
libsecurity_utilities/lib/macho++.cpp|643 col 6| mSuspicious = true;
libsecurity_utilities/lib/macho++.cpp|654 col 5| mSuspicious = true;

According to MachO::validateStructure

  1. The sum of the last LoadCommand and its size must equal to the total size of "fat" component,
    which sometimes corresponds to the end of file.

According to Universal::Universal

  1. Alignment of "fat" components must be less than 2^30, and
  2. The gap size between "fat" components must be less than its alignment.
  3. The gap must be filled with zero bytes.
  4. No junk shall be past end of file or "fat" component.

Relevant requirement

From the above there should be no data in MachO past contents of a load command with the largest offset+size.

Problem description

llvm-objcopy does not update indirectsymoff field if the resulting IndirectSymTable (after stripping) is empty [1]. As a result indirectsymoff keeps old value, which is garbage, and can point past offset+size of the last load command. As a result totalSize() returns size that is larger than the actual stripped MachO [2].

This clearly violates one of the checks mentioned above (there are zero-bytes filled past offset+size of the last load command.

@RIscRIpt RIscRIpt linked a pull request Nov 26, 2024 that will close this issue
RIscRIpt pushed a commit to RIscRIpt/llvm-project that referenced this issue Nov 29, 2024
Let's say we've run llvm-strip over some MachO. The resulting MachO
became smaller and there are no indirect symbols anymore.

Then according to previous code we would not update indirectsymoff
field. This would lead to `MachOWriter::totalSize()` report size that is
larger than the new MachO. As a result we would get MachO that has zero
bytes past contents of the very last load command.

Codesign has a strict check that size of MachO file must be equal to

    lastLoadCommand.offset + lastLoadCommand.size

If this is not satisfied codesign reports the following error

    main executable failed strict validation

Fixes llvm#117723
RIscRIpt added a commit that referenced this issue Nov 29, 2024
Let's say we've run llvm-strip over some MachO. The resulting MachO
became smaller and there are no indirect symbols anymore.

Then according to previous code we would not update indirectsymoff
field. This would lead to `MachOWriter::totalSize()` report size that is
larger than the new MachO. As a result we would get MachO that has zero
bytes past contents of the very last load command.

Codesign has a strict check that size of MachO file must be equal to

    lastLoadCommand.offset + lastLoadCommand.size

If this is not satisfied codesign reports the following error

    main executable failed strict validation

Fixes #117723
@EugeneZelenko EugeneZelenko removed the bug Indicates an unexpected problem or unintended behavior label Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants