Skip to content

feat: use objcopy instead of LIEF #64

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

Closed
RaisinTen wants to merge 2 commits into from
Closed

feat: use objcopy instead of LIEF #64

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4441c2e
feat: use objcopy instead of LIEF
RaisinTen Nov 3, 2022
bb3bed8
fix: stop using template string for exec
RaisinTen Nov 4, 2022
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
2 changes: 1 addition & 1 deletion README.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Options:
```js
const { inject } = require('postject');

await inject('a.out', 'lol', Buffer.from('Hello, world!'));
await inject('a.out', 'lol', '/path/to/resource/file');
```

## Building
Expand Down
41 changes: 14 additions & 27 deletions src/api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
const { constants, promises: fs } = require("fs");
const path = require("path");
const util = require("util");
const execFile = util.promisify(require("child_process").execFile);

const loadPostjectModule = require("./postject.js");

async function inject(filename, resourceName, resourceData, options) {
async function inject(filename, resourceName, resource, options) {
const machoSegmentName = options?.machoSegmentName || "__POSTJECT";
const overwrite = options?.overwrite || false;

if (!Buffer.isBuffer(resourceData)) {
throw new TypeError("resourceData must be a buffer");
let resourceData;
try {
resourceData = await fs.readFile(resource);
} catch {
throw new Error("Can't access resource file");
}

try {
Expand Down Expand Up @@ -39,30 +44,12 @@ async function inject(filename, resourceName, resourceData, options) {

switch (executableFormat) {
case postject.ExecutableFormat.kMachO:
{
let sectionName = resourceName;

// Mach-O section names are conventionally of the style __foo
if (!sectionName.startsWith("__")) {
sectionName = `__${sectionName}`;
}

({ result, data } = postject.injectIntoMachO(
executable,
machoSegmentName,
sectionName,
resourceData,
overwrite
));

if (result === postject.InjectResult.kAlreadyExists) {
throw new Error(
`Segment and section with that name already exists: ${machoSegmentName}/${sectionName}\n` +
"Use --overwrite to overwrite the existing content"
);
}
Comment on lines -58 to -63
Copy link
Contributor

@dsanders11 dsanders11 Nov 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes in this PR break --overwrite since it doesn't check if the section exists.

I believe you'd want to use llvm-objcopy --update-section to get the overwrite functionality.

}
break;
await execFile("/usr/local/opt/llvm/bin/llvm-objcopy", [
Copy link
Contributor

@dsanders11 dsanders11 Nov 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This path shouldn't be hardcoded, since it depends on the user's system. For example, on my system after doing brew install llvm, I do not have that path, it says it didn't link it since there's already system binaries which it would conflict with.

The more robust option would be to try to find the binary, and also allow an option or environment variable to override the path.

"--add-section",
`${machoSegmentName},__${resourceName}=${resource}`,
filename,
]);
return;

case postject.ExecutableFormat.kELF:
{
Expand Down
7 changes: 2 additions & 5 deletions src/cli.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,15 @@ async function main(filename, resourceName, resource, options) {
process.exit();
}

let resourceData;

try {
await fs.access(resource, constants.R_OK);
resourceData = await fs.readFile(resource);
} catch {
console.log("Can't read resource file");
console.log("Can't access resource file");
process.exit(1);
}

try {
await inject(filename, resourceName, resourceData, {
await inject(filename, resourceName, resource, {
machoSegmentName: options.machoSegmentName,
overwrite: options.overwrite,
});
Expand Down
66 changes: 0 additions & 66 deletions src/postject.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,71 +84,6 @@ emscripten::val inject_into_elf(const emscripten::val& executable,
return object;
}

emscripten::val inject_into_macho(const emscripten::val& executable,
const std::string& segment_name,
const std::string& section_name,
const emscripten::val& data,
bool overwrite = false) {
emscripten::val object = emscripten::val::object();
object.set("data", emscripten::val::undefined());

std::unique_ptr<LIEF::MachO::FatBinary> fat_binary =
LIEF::MachO::Parser::parse(vec_from_val(executable));

if (!fat_binary) {
object.set("result", emscripten::val(InjectResult::kError));
return object;
}

// Inject into all Mach-O binaries if there's more than one in a fat binary
for (LIEF::MachO::Binary& binary : *fat_binary) {
LIEF::MachO::Section* existing_section =
binary.get_section(segment_name, section_name);

if (existing_section) {
if (!overwrite) {
object.set("result", emscripten::val(InjectResult::kAlreadyExists));
return object;
}

binary.remove_section(segment_name, section_name, true);
}

LIEF::MachO::SegmentCommand* segment = binary.get_segment(segment_name);
LIEF::MachO::Section section(section_name, vec_from_val(data));

if (!segment) {
// Create the segment and mark it read-only
LIEF::MachO::SegmentCommand new_segment(segment_name);
new_segment.max_protection(
static_cast<uint32_t>(LIEF::MachO::VM_PROTECTIONS::VM_PROT_READ));
new_segment.init_protection(
static_cast<uint32_t>(LIEF::MachO::VM_PROTECTIONS::VM_PROT_READ));
new_segment.add_section(section);
Comment on lines -121 to -127
Copy link
Contributor

@dsanders11 dsanders11 Nov 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR doesn't mark the segment as read-only, instead the segment will get the default protections, read/write/execute. Part of the security model of Postject is that the resources are injected as read-only to ensure they aren't at risk of being modified in a running executable, which the OS providing that automatically since the loaded segments are read-only and so the virtual memory pages will be read-only.

There is a --set-section-flags option which could be used simultaneously to make the segment read-only, but trying to use it on a Mach-O executable tells me: llvm-objcopy: error: option is not supported for MachO.

binary.add(new_segment);
} else {
binary.add_section(*segment, section);
}

// It will need to be signed again anyway, so remove the signature
if (binary.has_code_signature()) {
binary.remove_signature();
}
}

// Construct a new Uint8Array in JS
std::vector<uint8_t> output = fat_binary->raw();
emscripten::val view{
emscripten::typed_memory_view(output.size(), output.data())};
auto output_data = emscripten::val::global("Uint8Array").new_(output.size());
output_data.call<void>("set", view);

object.set("data", output_data);
object.set("result", emscripten::val(InjectResult::kSuccess));

return object;
}

emscripten::val inject_into_pe(const emscripten::val& executable,
const std::string& resource_name,
const emscripten::val& data,
Expand Down Expand Up @@ -286,6 +221,5 @@ EMSCRIPTEN_BINDINGS(postject) {
.value("kSuccess", InjectResult::kSuccess);
emscripten::function("getExecutableFormat", &get_executable_format);
emscripten::function("injectIntoELF", &inject_into_elf);
emscripten::function("injectIntoMachO", &inject_into_macho);
emscripten::function("injectIntoPE", &inject_into_pe);
}
39 changes: 35 additions & 4 deletions test/cli.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,23 @@ describe("postject CLI", () => {
expect(stdout).to.have.string("Hello world");
}

// Code signing using a self-signed certificate.
{
if (process.platform === "darwin") {
let codesignFound = false;
try {
execSync("command -v codesign");
codesignFound = true;
} catch (err) {
console.log(err.message);
}
if (codesignFound) {
execSync(`codesign --sign - ${filename}`);
}
}
// TODO(RaisinTen): Test code signing on Windows.
}

{
const { status, stdout, stderr } = spawnSync(
"node",
Expand All @@ -95,7 +112,6 @@ describe("postject CLI", () => {
console.log(err.message);
}
if (codesignFound) {
execSync(`codesign --sign - ${filename}`);
Copy link
Member Author

@RaisinTen RaisinTen Nov 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason, code signing the binary after running objcopy --add-section on macOS causes the binary to crash:

$ ./node
[1]    5366 killed     ./node
$ echo $?
137

and I can't even lldb into it:

lldb -- ./node
(lldb) target create "./node"
Current executable set to '/Users/raisinten/Desktop/git/postject/node' (x86_64).
(lldb) run
error: Cannot allocate memory
(lldb) ^D

However, it seems to be possible to run objcopy after code signing and not invalidate the code signature! I also tried to check if the new section was being added after the LC_CODE_SIGNATURE section that @bnoordhuis mentioned in nodejs/node#45066 (comment) and it seems like objcopy adds new sections at the end, even after LC_CODE_SIGNATURE but the codesign tool thinks that this is valid:

$ otool -l ./node | tail -n29
Load command 18
      cmd LC_CODE_SIGNATURE
  cmdsize 16
  dataoff 84875008
 datasize 663232
Load command 19
      cmd LC_SEGMENT_64
  cmdsize 152
  segname __POSTJECT
   vmaddr 0x00000001057d4000
   vmsize 0x0000000000001000
  fileoff 68509696
 filesize 4096
  maxprot 0x00000007
 initprot 0x00000007
   nsects 1
    flags 0x0
Section
  sectname __NODE_JS_CODE
   segname __POSTJECT
      addr 0x00000001057d4000
      size 0x0000000000000014
    offset 68509696
     align 2^0 (1)
    reloff 0
    nreloc 0
     flags 0x00000000
 reserved1 0
 reserved2 0
$ codesign -v ./node
$ echo $?
0

Any clue what's happening here?

Copy link
Contributor

@dsanders11 dsanders11 Nov 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason, code signing the binary after running objcopy --add-section on macOS causes the binary to crash

A binary with segments after LC_CODE_SIGNATURE is not valid for code signing. There are certain requirements for a Mach-O executable being signable, usually when they're not met the codesign tool you get the error "main executable failed strict validation", not sure why it's not happening in this case.

[1] 5366 killed ./node

If you check dmesg you can get more information on why it was killed. On macOS when an executable is killed like that immediately on start (killed, not crashing) then it is probably the kernel killing it, and there will be a message logged as to why it killed it.

I wasn't able to reproduce this case (I can see the segment is after LC_CODE_SIGNATURE) in a quick test, but if you can reliably reproduce, check dmesg to find more information on why it was killed.

execSync(`codesign --verify ${filename}`);
}
}
Expand Down Expand Up @@ -151,9 +167,25 @@ describe("postject API", () => {
expect(stdout).to.have.string("Hello world");
}

// Code signing using a self-signed certificate.
{
const resourceData = await fs.readFile(resourceFilename);
await inject(filename, "foobar", resourceData);
if (process.platform === "darwin") {
let codesignFound = false;
try {
execSync("command -v codesign");
codesignFound = true;
} catch (err) {
console.log(err.message);
}
if (codesignFound) {
execSync(`codesign --sign - ${filename}`);
}
}
// TODO(RaisinTen): Test code signing on Windows.
}

{
await inject(filename, "foobar", resourceFilename);
}

// Verifying code signing using a self-signed certificate.
Expand All @@ -167,7 +199,6 @@ describe("postject API", () => {
console.log(err.message);
}
if (codesignFound) {
execSync(`codesign --sign - ${filename}`);
execSync(`codesign --verify ${filename}`);
}
}
Expand Down