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

codeql-action/upload-sarif - JavaScript heap out of memory #528

Closed
jsjoeio opened this issue May 24, 2021 · 5 comments · Fixed by #550
Closed

codeql-action/upload-sarif - JavaScript heap out of memory #528

jsjoeio opened this issue May 24, 2021 · 5 comments · Fixed by #550
Assignees

Comments

@jsjoeio
Copy link

jsjoeio commented May 24, 2021

We noticed recently github/codeql-action/upload-sarif@v1 failing in our CI.

Run github/codeql-action/upload-sarif@v1
  with:
    sarif_file: trivy-image-results.sarif
    checkout_path: /home/runner/work/code-server/code-server
    token: ***
    matrix: null
Uploading sarif files: ["trivy-image-results.sarif"]
FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory

<--- Last few GCs --->

[2047:0x42960e0]   211606 ms: Mark-sweep 1968.4 (1973.9) -> 1968.4 (1970.9) MB, 1146.9 / 0.0 ms  (average mu = 0.902, current mu = 0.000) last resort GC in old space requested
[2047:0x42960e0]   212790 ms: Mark-sweep 1968.4 (1970.9) -> 1968.4 (1970.9) MB, 1183.6 / 0.0 ms  (average mu = 0.811, current mu = 0.000) last resort GC in old space requested


<--- JS stacktrace --->

==== JS stack trace =========================================

    0: ExitFrame [pc: 0x1374fd9]
Security context: 0x1df0d8ac08a1 <JSObject>
    1: outputHash(aka outputHash) [0x23868395bba1] [/home/runner/work/_actions/github/codeql-action/v1/lib/fingerprints.js:~63] [pc=0x4e7bc427b85](this=0x2d8a3d4404a9 <undefined>)
    2: hash(aka hash) [0x4b2344f8471] [/home/runner/work/_actions/github/codeql-action/v1/lib/fingerprints.js:~41] [pc=0x4e7bc42400b](this=0x2d8a3d4404a9 <undefined>,0x23868395bbe1 <JSF...

 1: 0x9da7c0 node::Abort() [/home/runner/runners/2.278.0/externals/node12/bin/node]
 2: 0x9db976 node::OnFatalError(char const*, char const*) [/home/runner/runners/2.278.0/externals/node12/bin/node]
 3: 0xb39f1e v8::Utils::ReportOOMFailure(v8::internal::Isolate*, char const*, bool) [/home/runner/runners/2.278.0/externals/node12/bin/node]
 4: 0xb3a299 v8::internal::V8::FatalProcessOutOfMemory(v8::internal::Isolate*, char const*, bool) [/home/runner/runners/2.278.0/externals/node12/bin/node]
 5: 0xce5635  [/home/runner/runners/2.278.0/externals/node12/bin/node]
 6: 0xcf54fd v8::internal::Heap::AllocateRawWithRetryOrFail(int, v8::internal::AllocationType, v8::internal::AllocationAlignment) [/home/runner/runners/2.278.0/externals/node12/bin/node]
 7: 0xcbc118 v8::internal::Factory::NewFixedArrayWithFiller(v8::internal::RootIndex, int, v8::internal::Object, v8::internal::AllocationType) [/home/runner/runners/2.278.0/externals/node12/bin/node]
 8: 0xcbc220 v8::internal::Handle<v8::internal::FixedArray> v8::internal::Factory::NewFixedArrayWithMap<v8::internal::FixedArray>(v8::internal::RootIndex, int, v8::internal::AllocationType) [/home/runner/runners/2.278.0/externals/node12/bin/node]
 9: 0xecb760 v8::internal::HashTable<v8::internal::StringTable, v8::internal::StringTableShape>::NewInternal(v8::internal::Isolate*, int, v8::internal::AllocationType) [/home/runner/runners/2.278.0/externals/node12/bin/node]
10: 0xecb7be v8::internal::HashTable<v8::internal::StringTable, v8::internal::StringTableShape>::New(v8::internal::Isolate*, int, v8::internal::AllocationType, v8::internal::MinimumCapacity) [/home/runner/runners/2.278.0/externals/node12/bin/node]
11: 0xecbe8b v8::internal::HashTable<v8::internal::StringTable, v8::internal::StringTableShape>::EnsureCapacity(v8::internal::Isolate*, v8::internal::Handle<v8::internal::StringTable>, int, v8::internal::AllocationType) [/home/runner/runners/2.278.0/externals/node12/bin/node]
12: 0xed4af1 v8::internal::Handle<v8::internal::String> v8::internal::StringTable::LookupKey<v8::internal::InternalizedStringKey>(v8::internal::Isolate*, v8::internal::InternalizedStringKey*) [/home/runner/runners/2.278.0/externals/node12/bin/node]
13: 0xed4be6 v8::internal::StringTable::LookupString(v8::internal::Isolate*, v8::internal::Handle<v8::internal::String>) [/home/runner/runners/2.278.0/externals/node12/bin/node]
14: 0x100b30b v8::internal::Runtime_GetProperty(int, unsigned long*, v8::internal::Isolate*) [/home/runner/runners/2.278.0/externals/node12/bin/node]
15: 0x1374fd9  [/home/runner/runners/2.278.0/externals/node12/bin/node]

See logs.

Are there any workarounds or fixes for this? (besides upgrading to a custom CI runner)

Possibly Related:

@adityasharad
Copy link
Contributor

Thanks for letting us know. Would you be able to share a snippet of the SARIF file that is being uploaded, and let us know how large the file is?

What I think is happening here is that the SARIF file is too large for the Node.js process to hold in memory. The failing step processes the SARIF in memory and adds fingerprints to the result objects. Even if we got past this processing stage, it's possible it may be too large for the Code Scanning API to accept (there is a 2GB limit). Some options to consider for reducing SARIF size include adjusting which files are being scanned by the tool, or configuring whether to include file snippets in the SARIF.

@jsjoeio
Copy link
Author

jsjoeio commented May 24, 2021

Sure! Let me see if I can get that for you quickly.

@jsjoeio
Copy link
Author

jsjoeio commented May 24, 2021

Snippet:

{
  "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json",
  "version": "2.1.0",
  "runs": [
    {
      "tool": {
        "driver": {
          "name": "Trivy",
          "informationUri": "https://github.com/aquasecurity/trivy",
          "fullName": "Trivy Vulnerability Scanner",
          "version": "0.15.0",
          "rules": [
            {
              "id": "./release-images/code-server-amd64-3.10.2.tar (debian 10.9): liblz4-1-1.8.3-1 CVE-2021-3520",
              "name": "OS Package Vulnerability (Debian)",
              "shortDescription": {
                "text": "CVE-2021-3520 Package: liblz4-1"
              },
              "fullDescription": {
                "text": "lz4: memory corruption due to an integer overflow bug caused by memmove argument."
              },
              "defaultConfiguration": {
                "level": "error"
              },
              "helpUri": "https://avd.aquasec.com/nvd/cve-2021-3520",
              "help": {
                "text": "Vulnerability CVE-2021-3520\nSeverity: HIGH\nPackage: liblz4-1\nInstalled Version: 1.8.3-1\nFixed Version: 1.8.3-1+deb10u1\nLink: [CVE-2021-3520](https://avd.aquasec.com/nvd/cve-2021-3520)",
                "markdown": "**Vulnerability CVE-2021-3520**\n| Severity | Package | Installed Version | Fixed Version | Link |\n| --- | --- | --- | --- | --- |\n|HIGH|liblz4-1|1.8.3-1|1.8.3-1+deb10u1|[CVE-2021-3520](https://avd.aquasec.com/nvd/cve-2021-3520)|\n"
              },
              "properties": {
                "tags": [
                  "vulnerability",
                  "HIGH",
                  "liblz4-1"
                ],
                "precision": "very-high"
              }
            },
            {
              "id": "usr/lib/code-server/lib/coder-cloud-agent: golang.org/x/crypto-v0.0.0-20191206172530-e9b2fee46413 CVE-2020-29652",
              "name": "Other Vulnerability (Gobinary)",
              "shortDescription": {
                "text": "CVE-2020-29652 Package: golang.org/x/crypto"
              },
              "fullDescription": {
                "text": "golang: crypto/ssh: crafted authentication request can lead to nil pointer dereference."
              },
              "defaultConfiguration": {
                "level": "error"
              },
              "helpUri": "https://avd.aquasec.com/nvd/cve-2020-29652",
              "help": {
                "text": "Vulnerability CVE-2020-29652\nSeverity: HIGH\nPackage: golang.org/x/crypto\nInstalled Version: v0.0.0-20191206172530-e9b2fee46413\nFixed Version: v0.0.0-20201216223049-8b5274cf687f\nLink: [CVE-2020-29652](https://avd.aquasec.com/nvd/cve-2020-29652)",
                "markdown": "**Vulnerability CVE-2020-29652**\n| Severity | Package | Installed Version | Fixed Version | Link |\n| --- | --- | --- | --- | --- |\n|HIGH|golang.org/x/crypto|v0.0.0-20191206172530-e9b2fee46413|v0.0.0-20201216223049-8b5274cf687f|[CVE-2020-29652](https://avd.aquasec.com/nvd/cve-2020-29652)|\n"
              },
              "properties": {
                "tags": [
                  "vulnerability",
                  "HIGH",
                  "golang.org/x/crypto"
                ],
                "precision": "very-high"
              }
            },
            {
              "id": "usr/lib/code-server/lib/coder-cloud-agent: golang.org/x/crypto-v0.0.0-20191206172530-e9b2fee46413 CVE-2020-9283",
              "name": "Other Vulnerability (Gobinary)",
              "shortDescription": {
                "text": "CVE-2020-9283 Package: golang.org/x/crypto"
              },
              "fullDescription": {
                "text": "golang.org/x/crypto: Processing of crafted ssh-ed25519 public keys allows for panic."
              },
              "defaultConfiguration": {
                "level": "error"
              },
              "helpUri": "https://avd.aquasec.com/nvd/cve-2020-9283",
              "help": {
                "text": "Vulnerability CVE-2020-9283\nSeverity: HIGH\nPackage: golang.org/x/crypto\nInstalled Version: v0.0.0-20191206172530-e9b2fee46413\nFixed Version: v0.0.0-20200220183623-bac4c82f6975\nLink: [CVE-2020-9283](https://avd.aquasec.com/nvd/cve-2020-9283)",
                "markdown": "**Vulnerability CVE-2020-9283**\n| Severity | Package | Installed Version | Fixed Version | Link |\n| --- | --- | --- | --- | --- |\n|HIGH|golang.org/x/crypto|v0.0.0-20191206172530-e9b2fee46413|v0.0.0-20200220183623-bac4c82f6975|[CVE-2020-9283](https://avd.aquasec.com/nvd/cve-2020-9283)|\n"
              },
              "properties": {
                "tags": [
                  "vulnerability",
                  "HIGH",
                  "golang.org/x/crypto"
                ],
                "precision": "very-high"
              }
            }]
        }
      },
      "results": [
        {
          "ruleId": "./release-images/code-server-amd64-3.10.2.tar (debian 10.9): liblz4-1-1.8.3-1 CVE-2021-3520",
          "ruleIndex": 0,
          "level": "error",
          "message": {
            "text": "There&#39;s a flaw in lz4. An attacker who submits a crafted file to an application linked with lz4 may be able to trigger an integer overflow, leading to calling of memmove() on a negative size argument, causing an out-of-bounds write and/or a crash. The greatest impact of this flaw is to availability, with some potential impact to confidentiality and integrity as well."
          },
          "locations": [{
            "physicalLocation": {
              "artifactLocation": {
                "uri": "./release-images/code-server-amd64-3.10.2.tar",
                "uriBaseId": "ROOTPATH"
              }
            }
          }]
        },
        {
          "ruleId": "usr/lib/code-server/lib/coder-cloud-agent: golang.org/x/crypto-v0.0.0-20191206172530-e9b2fee46413 CVE-2020-29652",
          "ruleIndex": 0,
          "level": "error",
          "message": {
            "text": "A nil pointer dereference in the golang.org/x/crypto/ssh component through v0.0.0-20201203163018-be400aefbc4c for Go allows remote attackers to cause a denial of service against SSH servers."
          },
          "locations": [{
            "physicalLocation": {
              "artifactLocation": {
                "uri": "usr/lib/code-server/lib/coder-cloud-agent",
                "uriBaseId": "ROOTPATH"
              }
            }
          }]
        },
        {
          "ruleId": "usr/lib/code-server/lib/coder-cloud-agent: golang.org/x/crypto-v0.0.0-20191206172530-e9b2fee46413 CVE-2020-9283",
          "ruleIndex": 1,
          "level": "error",
          "message": {
            "text": "golang.org/x/crypto before v0.0.0-20200220183623-bac4c82f6975 for Go allows a panic during signature verification in the golang.org/x/crypto/ssh package. A client can attack an SSH server that accepts public keys. Also, a server can attack any SSH client."
          },
          "locations": [{
            "physicalLocation": {
              "artifactLocation": {
                "uri": "usr/lib/code-server/lib/coder-cloud-agent",
                "uriBaseId": "ROOTPATH"
              }
            }
          }]
        }],
      "columnKind": "utf16CodeUnits",
      "originalUriBaseIds": {
        "ROOTPATH": {
          "uri": "/"
        }
      }
    }
  ]
}

File size: 7.3kb

-rw-r--r-- 1 root root 7375 May 24 18:52 trivy-image-results.sarif

See logs.

@adityasharad
Copy link
Contributor

adityasharad commented May 26, 2021

Thank you for the detailed information. Where possible, the Action computes a fingerprint for each result, by hashing the source file at the location of the result, and inserts that fingerprint into the SARIF. In this case it looks like we are attempting to hash the large TAR file that was scanned in your workflow, and that's too large for Node to hold in memory.

I think there are two approaches here:

  • Short term workaround: please try removing the TAR file /release-images/code-server-amd64-3.10.2.tar (and others like it) from the workspace before running the upload step.
  • Longer term solution for us to implement: we should not attempt to hash files for results that do not have a line number location, because those are unlikely to be source files and won't produce a consistent fingerprint.

@RA80533
Copy link
Contributor

RA80533 commented Jun 5, 2021

This is actually the perfect use case for Node.js to handle, especially due to how the hash function for fingerprinting was implemented (it's a rolling hash); however, files would have to be handled a bit differently in order for this to happen.

Node.js is backed by a wonderful I/O library called libuv which excels in asynchrony. One of its capabilities is file streaming which Node.js takes advantage of to tremendous success thanks to its event-driven design. Right now, fingerprinting requires files to be loaded in their entirety. Obviously, large files will kill Node.js due to memory constraints if they're handled this way but, as seen in this thread, smaller files will too (the archive is only ~60 MB).

Related #488

@edoardopirovano edoardopirovano self-assigned this Jun 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants