Merge pull request #320 from snyk/fix/try-catch-go-parsing

Fix/try catch go parsing
snyk · Feb 9, 2021 · 727ced7 · 727ced7
2 parents fac3665 + aca5829
commit 727ced7
Show file tree

Hide file tree

Showing 3 changed files with 98 additions and 58 deletions.
diff --git a/lib/extractor/layer.ts b/lib/extractor/layer.ts
@@ -1,10 +1,13 @@
+import * as Debug from "debug";
 import * as gunzip from "gunzip-maybe";
 import * as path from "path";
 import { Readable } from "stream";
 import { extract, Extract } from "tar-stream";
 import { applyCallbacks, isResultEmpty } from "./callbacks";
 import { ExtractAction, ExtractedLayers } from "./types";
 
+const debug = Debug("snyk");
+
 /**
  * Extract key files from the specified TAR stream.
  * @param layerTarStream image layer as a Readable TAR stream. Note: consumes the stream.
@@ -26,14 +29,23 @@ export async function extractImageLayer(
           action.filePathMatches(absoluteFileName),
         );
         if (matchedActions.length > 0) {
-          const callbackResult = await applyCallbacks(
-            matchedActions,
-            stream,
-            headers.size,
-          );
-
-          if (!isResultEmpty(callbackResult)) {
-            result[absoluteFileName] = callbackResult;
+          try {
+            const callbackResult = await applyCallbacks(
+              matchedActions,
+              stream,
+              headers.size,
+            );
+
+            if (!isResultEmpty(callbackResult)) {
+              result[absoluteFileName] = callbackResult;
+            }
+          } catch (error) {
+            // An ExtractAction has thrown an uncaught exception, likely a bug in the code!
+            debug(
+              "Exception thrown while applying callbacks during image layer extraction",
+              JSON.stringify(error),
+            );
+            reject(error);
           }
         }
       }

diff --git a/lib/go-parser/index.ts b/lib/go-parser/index.ts
@@ -60,58 +60,62 @@ async function findGoBinaries(
     let bytesWritten = 0;
 
     stream.on("end", () => {
-      // Discard
-      if (bytesWritten === 0) {
-        return resolve();
-      }
-
-      const binaryFile = elf.parse(buffer);
-
-      const goBuildInfo = binaryFile.body.sections.find(
-        (section) => section.name === ".go.buildinfo",
-      );
-      // Could be found in file headers
-      const goBuildId = binaryFile.body.sections.find(
-        (section) => section.name === ".note.go.buildid",
-      );
-
-      const interp = binaryFile.body.sections.find(
-        (section) => section.name === ".interp",
-      );
-
-      if (!goBuildInfo && !goBuildId) {
-        return resolve();
-      } else if (interp) {
-        // Compiled using cgo
-        // we wouldn't be able to extract modules
-        // TODO: cgo-compiled binaries are not supported in this iteration
-        return resolve();
-      } else if (goBuildInfo) {
-        const info = goBuildInfo.data
-          .slice(0, buildInfoMagic.length)
-          .toString(encoding);
-
-        if (info === buildInfoMagic) {
-          return resolve(binaryFile);
+      try {
+        // Discard
+        if (bytesWritten === 0) {
+          return resolve();
         }
 
-        return resolve();
-      } else if (goBuildId) {
-        const strings = goBuildId.data
-          .toString()
-          .split(/\0+/g)
-          .filter(Boolean);
-        const go = strings[strings.length - 2];
-        const buildIdParts = strings[strings.length - 1].split("/");
-
-        // Build ID's precise form is actionID/[.../]contentID.
-        // Usually the buildID is simply actionID/contentID, but with exceptions.
-        // https://github.com/golang/go/blob/master/src/cmd/go/internal/work/buildid.go#L23
-        if (go === buildIdMagic && buildIdParts.length >= 2) {
-          return resolve(binaryFile);
+        const binaryFile = elf.parse(buffer);
+
+        const goBuildInfo = binaryFile.body.sections.find(
+          (section) => section.name === ".go.buildinfo",
+        );
+        // Could be found in file headers
+        const goBuildId = binaryFile.body.sections.find(
+          (section) => section.name === ".note.go.buildid",
+        );
+
+        const interp = binaryFile.body.sections.find(
+          (section) => section.name === ".interp",
+        );
+
+        if (!goBuildInfo && !goBuildId) {
+          return resolve();
+        } else if (interp) {
+          // Compiled using cgo
+          // we wouldn't be able to extract modules
+          // TODO: cgo-compiled binaries are not supported in this iteration
+          return resolve();
+        } else if (goBuildInfo) {
+          const info = goBuildInfo.data
+            .slice(0, buildInfoMagic.length)
+            .toString(encoding);
+
+          if (info === buildInfoMagic) {
+            return resolve(binaryFile);
+          }
+
+          return resolve();
+        } else if (goBuildId) {
+          const strings = goBuildId.data
+            .toString()
+            .split(/\0+/g)
+            .filter(Boolean);
+          const go = strings[strings.length - 2];
+          const buildIdParts = strings[strings.length - 1].split("/");
+
+          // Build ID's precise form is actionID/[.../]contentID.
+          // Usually the buildID is simply actionID/contentID, but with exceptions.
+          // https://github.com/golang/go/blob/master/src/cmd/go/internal/work/buildid.go#L23
+          if (go === buildIdMagic && buildIdParts.length >= 2) {
+            return resolve(binaryFile);
+          }
+
+          return resolve();
         }
-
-        return resolve();
+      } catch (error) {
+        reject(error);
       }
     });
 

diff --git a/test/system/application-scans/gomodules.spec.ts b/test/system/application-scans/gomodules.spec.ts
@@ -1,7 +1,13 @@
-import { scan } from "../../../lib";
+import * as elf from "elfy";
+
+import { PluginResponse, scan } from "../../../lib";
 import { getFixture } from "../../util";
 
 describe("gomodules binaries scanning", () => {
+  afterAll(() => {
+    jest.resetAllMocks();
+  });
+
   it("should return expected result", async () => {
     // Arrange
     const fixturePath = getFixture("docker-archives/docker-save/yq.tar");
@@ -16,4 +22,22 @@ describe("gomodules binaries scanning", () => {
     // Assert
     expect(pluginResult).toMatchSnapshot();
   });
+
+  it("throws an error when a Go binary cannot be parsed", async () => {
+    const elfParseMock = jest.spyOn(elf, "parse").mockImplementation(() => {
+      throw new Error("Cannot read property 'type' of undefined");
+    });
+
+    const fixturePath = getFixture("docker-archives/docker-save/yq.tar");
+    const imageNameAndTag = `docker-archive:${fixturePath}`;
+
+    await expect(() =>
+      scan({
+        path: imageNameAndTag,
+        "app-vulns": true,
+      }),
+    ).rejects.toThrow("Error reading tar archive");
+
+    elfParseMock.mockRestore();
+  });
 });