NVD import fails (panic) when description_data array is empty

iansltx commented 3 months ago

Fleet version: 4.38.0-4.55.0 (issue introduced in #13856)

💥 Actual behavior

Panic on pulling a vulnerability release containing CVEs with no entries in description_data, e.g. https://github.com/fleetdm/vulnerabilities/releases/tag/cve-202408092005. Shows up as a panic/exit on the explicit vuln_processing command, or a cron error when run via cron (see #21239).

level=info ts=2024-08-10T03:51:25.043954Z vulnerabilities=(MISSING) msg="scanning vulnerabilities"
level=info ts=2024-08-10T03:51:25.043984Z vulnerabilities=(MISSING) periodicity=1h0m0s
level=info ts=2024-08-10T03:51:25.044802Z vulnerabilities=(MISSING) msg="scanning vulnerabilities"
7panic: runtime error: index out of range [0] with length 0

goroutine 1 [running]:
github.com/fleetdm/fleet/v4/server/vulnerabilities/nvd.LoadCVEMeta({0x102cd6650, 0x14000a008d0}, {0x102cbeb00, 0x1400003c140}, {0x101eb72cb, 0xc}, {0x102d21eb8, 0x14000218780})
    /Users/ian/code/fleet/server/vulnerabilities/nvd/sync.go:221 +0xf5c
main.checkNVDVulnerabilities({0x102cd6650, 0x14000a008d0}, {0x102d21eb8, 0x14000218780}, {0x102cbeb00, 0x1400003c140}, {0x101eb72cb, 0xc}, 0x140006240e0, 0x0)
    /Users/ian/code/fleet/cmd/fleet/cron.go:385 +0x13c
main.scanVulnerabilities({0x102cd6650, 0x14000a008d0}, {0x102d21eb8, 0x14000218780}, {0x102cbeb00, 0x1400003c140}, 0x140006240e0, 0x14000650008, {0x101eb72cb, 0xc})
    /Users/ian/code/fleet/cmd/fleet/cron.go:160 +0x3dc
main.cronVulnerabilities({0x102cd6650, 0x14000a008d0}, {0x102d21eb8, 0x14000218780}, {0x102cbeb00, 0x1400003c140}, 0x140006240e0)
    /Users/ian/code/fleet/cmd/fleet/cron.go:99 +0x2e8
main.getVulnFuncs.func1({0x102cd6650?, 0x14000a008d0?})
    /Users/ian/code/fleet/cmd/fleet/vuln_process.go:165 +0x38
main.createVulnProcessingCmd.func1(0x140007c6f08, {0x101ea97e6?, 0x4?, 0x101ea97ea?})
    /Users/ian/code/fleet/cmd/fleet/vuln_process.go:115 +0x824
github.com/spf13/cobra.(*Command).execute(0x140007c6f08, {0x140000ac8a0, 0x3, 0x3})
    /Users/ian/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:983 +0x840
github.com/spf13/cobra.(*Command).ExecuteC(0x140007c6c08)
    /Users/ian/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1115 +0x344
github.com/spf13/cobra.(*Command).Execute(...)
    /Users/ian/go/pkg/mod/github.com/spf13/cobra@v1.8.0/command.go:1039
main.main()
    /Users/ian/code/fleet/cmd/fleet/main.go:64 +0x428

🧑‍💻 Steps to reproduce

Save the following as nvdcve-1.1-2024.json (the important part is missing description_data):

{
  "CVE_data_format": "MITRE",
  "CVE_data_numberOfCVEs": "18320",
  "CVE_data_timestamp": "2024-08-11T19:23:47Z",
  "CVE_data_type": "CVE",
  "CVE_data_version": "4.0",
  "CVE_Items": [
    {
      "cve": {
        "affects": null,
        "CVE_data_meta": {
          "ASSIGNER": "security@yugabyte.com",
          "ID": "CVE-2024-0006"
        },
        "data_format": "MITRE",
        "data_type": "CVE",
        "data_version": "4.0",
        "description": {
          "description_data": []
        },
        "problemtype": {
          "problemtype_data": []
        },
        "references": {
          "reference_data": [
            {
              "name": "https://github.com/yugabyte/yugabyte-db/commit/439c6286f1971f9ac6bff2c7215b454c2025c593",
              "url": "https://github.com/yugabyte/yugabyte-db/commit/439c6286f1971f9ac6bff2c7215b454c2025c593"
            },
            {
              "name": "https://github.com/yugabyte/yugabyte-db/commit/5cc7f4e15d6ccccbf97c57946fd0aa630f88c9e2",
              "url": "https://github.com/yugabyte/yugabyte-db/commit/5cc7f4e15d6ccccbf97c57946fd0aa630f88c9e2"
            },
            {
              "name": "https://github.com/yugabyte/yugabyte-db/commit/d96e6b629f34d065b47204daeeb44064e484c579",
              "url": "https://github.com/yugabyte/yugabyte-db/commit/d96e6b629f34d065b47204daeeb44064e484c579"
            }
          ]
        }
      },
      "configurations": {
        "CVE_data_version": "4.0",
        "nodes": [
          {
            "cpe_match": [
              {
                "cpe23Uri": "cpe:2.3:a:yugabyte:yugabytedb:*:*:*:*:*:*:*:*",
                "versionEndExcluding": "2.18.9.0",
                "versionStartIncluding": "2.18.0.0",
                "vulnerable": true
              },
              {
                "cpe23Uri": "cpe:2.3:a:yugabyte:yugabytedb:*:*:*:*:*:*:*:*",
                "versionEndExcluding": "2.20.2.3",
                "versionStartIncluding": "2.20.0.0",
                "vulnerable": true
              },
              {
                "cpe23Uri": "cpe:2.3:a:yugabyte:yugabytedb:*:*:*:*:*:*:*:*",
                "versionEndExcluding": "2024.1.1.0",
                "versionStartIncluding": "2024.0.0.0",
                "vulnerable": true
              }
            ],
            "operator": "OR"
          }
        ]
      },
      "impact": {},
      "lastModifiedDate": "2024-07-22T13:00Z",
      "publishedDate": "2024-07-19T15:15Z"
    }
  ]
}

gzip the above and place in /tmp/vulndbs (or place elsewhere and revise where the command picks up the vuln DB in step 4)
export FLEET_VULNERABILITIES_DISABLE_DATA_SYNC=true (to ensure we pick up the bugged file rather than pulling a more well-formed feed)
./build/fleet vuln_processing --dev --mysql_address=127.0.0.1:3310 --dev_license

You'll get a basically instant panic at this point.

🕯️ Timeline

2024-08-09 20:05 UTC - vulnerabilities artifactrs released containing empty description_data arrays for some CVEs in 2024 NVD file.
2024-08-09 ~8:45pm CT - In attempting to reproduce #20934, I see the panic on a local install when running fleet vuln_processing (macOS/Apple Silicon). Later that evening I repro the same issue on another machine (macOS/Intel).
2024-08-09 10:58pm CT - I start https://fleetdm.slack.com/archives/C019WG4GH0A/p1723262300008779 to get another set of eyes on my issue, figuring it was something I had set up incorrectly locally (I'm new here, and pretty new to golang).
2024-08-11 12:51am CT - I start https://fleetdm.slack.com/archives/C019WG4GH0A/p1723355495383819 after taking another look at the error earlier that evening, debugging through the panic and determining that the panic is caused by zero-length description_data arrays in CVE entries.
2024-08-11 1:02am CT - I verify that the patch mentioned in the "To fix" section solves the issue on a local install, as the CVE description field is skippable.
2024-08-11 1:12am CT - I note #13856 as the probable point where this latent bug started
2024-08-11 7:03am CT - Other team members hop on and start diagnosing the issue, leading to a fix in the upstream vulnerabilities database generation process (which doesn't require everyone to patch their Fleet servers with something that isn't even committed yet)

See #21239 for timeline on the vulnerabilities feed issue and fix.

🛠️ To fix

The following patch was manually confirmed to fix description handling when the array is empty (validated when the feed was still breaking unpatched clients):

Subject: [PATCH] Don't panic on zero-length NVD description_data array fields
---
Index: server/vulnerabilities/nvd/sync.go
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/server/vulnerabilities/nvd/sync.go b/server/vulnerabilities/nvd/sync.go
--- a/server/vulnerabilities/nvd/sync.go    (revision 11aaa4fed9082aefd905d5872a49c53a7824c74e)
+++ b/server/vulnerabilities/nvd/sync.go    (date 1723355931553)
@@ -216,9 +216,10 @@
            }
            schema := vuln.Schema()

-           meta := fleet.CVEMeta{
-               CVE:         cve,
-               Description: schema.CVE.Description.DescriptionData[0].Value,
+           meta := fleet.CVEMeta{CVE: cve}
+
+           if len(schema.CVE.Description.DescriptionData) > 0 {
+               meta.Description = schema.CVE.Description.DescriptionData[0].Value
            }

            if schema.Impact.BaseMetricV3 != nil {

Given that we don't have to rush a fix on this, and given that this problem is no longer trivially repro-able, the above needs to be augmented with tests that fail without the patch and succeed with it.

iansltx commented 3 months ago

Keeping this as :reproduce for the moment as, while I repro'd this last night, the feed fix means it takes more effort to repro. Will add repro steps when I have them.

iansltx commented 3 months ago

Repro'd by turning off pulling vulns via HTTP and placing a single description-less CVE entry in the vulndb dir (see updated repro steps).

Validating that my patch still works in this case now.

iansltx commented 3 months ago

Confirmed that the above fix works both on the trivial case (makes it past vuln processing when there's just the single JSON file) and on a fuller case (full feed with description of a CVE dropped). Going to get a PR up for this as we have a pretty straightforward manual test case and now that the upstream vuln feed is patched to always include at least one description entry there are probably better things to do than add an automated test on this (which looks like it would need to be an integration test, as the closest function call to the source of the issue would be LoadCVEMeta.

xpkoala commented 3 months ago

No panic encountered when testing this fix using the steps in the description.

fleet-release commented 3 months ago

Empty data array, patch brings peace to the code, Fleet now secure, whole.

fleetdm / fleet