Skip to content

Commit fe858e6

Browse files
authored
fix: file stream memory allocation bug (#707)
* fix: terminate early if file is not elf * chore: unit tests * fix: prevent overflow / pr comments * chore: update stream check
1 parent 10114d3 commit fe858e6

File tree

2 files changed

+156
-6
lines changed

2 files changed

+156
-6
lines changed

lib/go-parser/index.ts

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,13 +61,13 @@ async function findGoBinaries(
6161
// ELF section headers and so ".go.buildinfo" & ".note.go.buildid" blobs are available in the first 64kb
6262
const elfBuildInfoSize = 64 * 1024;
6363

64-
const buffer: Buffer = Buffer.alloc(streamSize ?? elfBuildInfoSize);
64+
let buffer: Buffer | null = null;
6565
let bytesWritten = 0;
6666

6767
stream.on("end", () => {
6868
try {
6969
// Discard
70-
if (bytesWritten === 0) {
70+
if (!buffer || bytesWritten === 0) {
7171
return resolve(undefined);
7272
}
7373

@@ -131,13 +131,31 @@ async function findGoBinaries(
131131
const first4Bytes = chunk.toString(encoding, 0, 4);
132132

133133
if (first4Bytes === elfHeaderMagic) {
134-
Buffer.from(chunk).copy(buffer, bytesWritten, 0);
135-
bytesWritten += chunk.length;
134+
// Now that we know it's an ELF file, allocate the buffer
135+
buffer = Buffer.alloc(streamSize ?? elfBuildInfoSize);
136+
137+
bytesWritten += Buffer.from(chunk).copy(buffer, bytesWritten, 0);
138+
136139
// Listen to next chunks only if it's an ELF executable
137140
stream.addListener("data", (chunk) => {
138-
Buffer.from(chunk).copy(buffer, bytesWritten, 0);
139-
bytesWritten += chunk.length;
141+
if (buffer && bytesWritten < buffer.length) {
142+
// Make sure we don't exceed the buffer capacity. Don't copy more
143+
// than the buffer can handle, and don't exceed the chunk length
144+
const bytesToWrite = Math.min(
145+
buffer.length - bytesWritten,
146+
chunk.length,
147+
);
148+
bytesWritten += Buffer.from(chunk).copy(
149+
buffer,
150+
bytesWritten,
151+
0,
152+
bytesToWrite,
153+
);
154+
}
140155
});
156+
} else {
157+
// Not an ELF file, exit early without allocating memory
158+
return resolve(undefined);
141159
}
142160
});
143161
});
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
import * as elf from "elfy";
2+
import { readFileSync } from "fs";
3+
import * as path from "path";
4+
import { Readable } from "stream";
5+
6+
import { getGoModulesContentAction } from "../../lib/go-parser";
7+
8+
// Helper to create a readable stream from buffer
9+
function createStreamFromBuffer(buffer: Buffer): Readable {
10+
const stream = new Readable();
11+
stream.push(buffer);
12+
stream.push(null);
13+
return stream;
14+
}
15+
16+
describe("Go parser memory allocation fix", () => {
17+
const findGoBinaries = getGoModulesContentAction.callback!;
18+
const { filePathMatches } = getGoModulesContentAction;
19+
20+
describe("Core memory fix - large non-ELF files", () => {
21+
it("should not allocate large buffers for non-ELF files", async () => {
22+
// This is the main bug we fixed - simulate 5GB non-ELF file
23+
const nonElfContent = Buffer.from("This is not an ELF file");
24+
const stream = createStreamFromBuffer(nonElfContent);
25+
const hugeReportedSize = 5 * 1024 * 1024 * 1024; // 5GB
26+
27+
// Act - should not cause memory allocation issues
28+
const result = await findGoBinaries(stream, hugeReportedSize);
29+
30+
// Assert
31+
expect(result).toBeUndefined();
32+
// If we reach here without memory errors, the fix works
33+
});
34+
35+
it("should still process legitimate ELF files", async () => {
36+
// Ensure we didn't break existing functionality
37+
const goBinaryPath = path.join(
38+
__dirname,
39+
"../fixtures/go-binaries/go1.18.5_normal",
40+
);
41+
42+
const goBinaryBuffer = readFileSync(goBinaryPath);
43+
const stream = createStreamFromBuffer(goBinaryBuffer);
44+
45+
const result = await findGoBinaries(stream, goBinaryBuffer.length);
46+
47+
// Should process ELF files
48+
expect(typeof result).toBeDefined();
49+
});
50+
});
51+
52+
describe("ELF magic detection", () => {
53+
it("should detect ELF magic correctly", async () => {
54+
const elfContent = Buffer.concat([
55+
Buffer.from("\x7FELF"), // ELF magic
56+
Buffer.alloc(100, 0),
57+
]);
58+
const stream = createStreamFromBuffer(elfContent);
59+
60+
// Mock elf.parse to avoid complexity
61+
const originalParse = elf.parse;
62+
elf.parse = jest.fn().mockReturnValue({ body: { sections: [] } });
63+
64+
const result = await findGoBinaries(stream, elfContent.length);
65+
66+
expect(elf.parse).toHaveBeenCalled();
67+
expect(result).toBeUndefined(); // No Go sections
68+
69+
elf.parse = originalParse;
70+
});
71+
72+
it("should reject non-ELF files early", async () => {
73+
const nonElfContent = Buffer.from("Not ELF content");
74+
const stream = createStreamFromBuffer(nonElfContent);
75+
76+
const result = await findGoBinaries(stream, 1024 * 1024); // 1MB
77+
78+
expect(result).toBeUndefined();
79+
});
80+
});
81+
82+
describe("filePathMatches function", () => {
83+
it("should match normal files without extensions", () => {
84+
expect(filePathMatches("/app/myservice")).toBe(true);
85+
expect(filePathMatches("/usr/bin/kubectl")).toBe(true);
86+
expect(filePathMatches("/opt/binary")).toBe(true);
87+
});
88+
89+
it("should not match files with extensions", () => {
90+
expect(filePathMatches("/app/script.sh")).toBe(false);
91+
expect(filePathMatches("/app/config.json")).toBe(false);
92+
expect(filePathMatches("/app/main.go")).toBe(false);
93+
});
94+
95+
it("should not match files in ignored paths", () => {
96+
expect(filePathMatches("/etc/passwd")).toBe(false);
97+
expect(filePathMatches("/var/log/app")).toBe(false);
98+
expect(filePathMatches("/tmp/file")).toBe(false);
99+
expect(filePathMatches("/proc/cpuinfo")).toBe(false);
100+
});
101+
});
102+
103+
describe("Error handling", () => {
104+
it("should catch stream errors", async () => {
105+
const errorStream = new Readable({
106+
read() {
107+
this.emit("error", new Error("Stream error"));
108+
},
109+
});
110+
111+
await expect(findGoBinaries(errorStream)).rejects.toThrow("Stream error");
112+
});
113+
114+
it("should return undefined for corrupted ELF files", async () => {
115+
const corruptedElf = Buffer.from("\x7FELF" + "corrupted");
116+
const stream = createStreamFromBuffer(corruptedElf);
117+
118+
const result = await findGoBinaries(stream);
119+
120+
expect(result).toBeUndefined(); // Should not throw
121+
});
122+
123+
it("should return undefined for empty streams", async () => {
124+
const emptyStream = new Readable();
125+
emptyStream.push(null);
126+
127+
const result = await findGoBinaries(emptyStream);
128+
129+
expect(result).toBeUndefined();
130+
});
131+
});
132+
});

0 commit comments

Comments
 (0)