Skip to content

fix(alpine): save not only CVE-IDs#532

Merged
knqyf263 merged 3 commits into
aquasecurity:mainfrom
DmitriyLewen:fix/alpine/ghsa-id-only
May 28, 2025
Merged

fix(alpine): save not only CVE-IDs#532
knqyf263 merged 3 commits into
aquasecurity:mainfrom
DmitriyLewen:fix/alpine/ghsa-id-only

Conversation

@DmitriyLewen

@DmitriyLewen DmitriyLewen commented May 26, 2025

Copy link
Copy Markdown
Contributor

Description

There are advisories with a vulnerability ID other than CVE-ID (e.g. GHSA-ID):
e.g. https://security.alpinelinux.org/srcpkg/vaultwarden

➜ grep -r "GHSA*" | grep -v CVE | sort -u
./3.10/main/squid.json:      "GHSA-572g-rvwr-6c7f"
./3.11/main/squid.json:      "GHSA-572g-rvwr-6c7f"
./3.12/main/squid.json:      "GHSA-572g-rvwr-6c7f"
./3.13/community/py3-bleach.json:      "GHSA-vv2x-vrpj-qqpq"
./3.13/main/glib.json:      "GHSL-2021-045",
./3.13/main/squid.json:      "GHSA-572g-rvwr-6c7f"
./3.14/community/py3-bleach.json:      "GHSA-vv2x-vrpj-qqpq"
./3.14/main/squid.json:      "GHSA-572g-rvwr-6c7f"
./3.15/community/py3-treq.json:      "GHSA-fhpf-pp6p-55qc"
./3.16/community/py3-treq.json:      "GHSA-fhpf-pp6p-55qc"
./3.17/community/py3-treq.json:      "GHSA-fhpf-pp6p-55qc"
./3.18/community/py3-treq.json:      "GHSA-fhpf-pp6p-55qc"
./3.19/community/py3-treq.json:      "GHSA-fhpf-pp6p-55qc"
./3.19/community/singularity.json:      "GHSA-77vh-xpmg-72qh"
./3.20/community/py3-treq.json:      "GHSA-fhpf-pp6p-55qc"
./3.20/community/singularity.json:      "GHSA-77vh-xpmg-72qh"
./3.21/community/py3-treq.json:      "GHSA-fhpf-pp6p-55qc"
./3.21/community/singularity.json:      "GHSA-77vh-xpmg-72qh"
./3.21/community/vaultwarden.json:      "GHSA-f7r5-w49x-gxm3",
./3.21/community/vaultwarden.json:      "GHSA-h6cc-rc6q-23j4",
./3.21/community/vaultwarden.json:      "GHSA-j4h8-vch3-f797"
./edge/community/py3-treq.json:      "GHSA-fhpf-pp6p-55qc"
./edge/community/singularity.json:      "GHSA-77vh-xpmg-72qh"
./edge/community/vaultwarden.json:      "GHSA-f7r5-w49x-gxm3",
./edge/community/vaultwarden.json:      "GHSA-h6cc-rc6q-23j4",
./edge/community/vaultwarden.json:      "GHSA-j4h8-vch3-f797"

So we don't need to save only CVE- advisories.

@DmitriyLewen DmitriyLewen self-assigned this May 26, 2025
@DmitriyLewen DmitriyLewen marked this pull request as ready for review May 26, 2025 10:48
@DmitriyLewen DmitriyLewen requested a review from knqyf263 as a code owner May 26, 2025 10:48
@knqyf263

Copy link
Copy Markdown
Collaborator

Interesting. Those GHSA-IDs have CVE-IDs. I'm wondering why they prefer GHSA-IDs.
GHSA-j4h8-vch3-f797

@DmitriyLewen

Copy link
Copy Markdown
Contributor Author

I asked about it - https://gitlab.alpinelinux.org/alpine/security/secdb/-/issues/11
Maybe they can just add the CVE-ID.

but as we know - a decrease in MITRE funding can lead to a delay between the appearance of a vulnerability (in this case, when the vulnerability is registered on GitHub (assigned an GHSA-ID)) and the moment when this vulnerability is assigned a CVE-ID.
that's why I think that regardless of this PR we need to be prepared for such situations

@DmitriyLewen

Copy link
Copy Markdown
Contributor Author

@knqyf263 Alpine answered my question - https://gitlab.alpinelinux.org/alpine/aports/-/issues/17193#note_511802
As I thought - there may be cases where the vulnerability didn't have a CVE-ID when they added this vulnerability (but that's a bit strange for a CVE from 2021).

So I think that such cases will arise, so we can add GHSA-IDS, but when Alpine updates CVE-ID - we will update advisory.

Comment thread pkg/vulnsrc/alpine/alpine.go Outdated
// Possible cases when vulnID contains only one non-CVE-ID
// e.g. only GHSA-ID
// cf. https://github.com/aquasecurity/vuln-list/blob/a830a1cc031f51a29a1a6f1c28d8366bbb5e7b64/alpine/3.21/community/vaultwarden.json#L9-L13
if !strings.HasPrefix(cveID, "CVE-") && len(ids) > 1 {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If I'm not wrong, I found two issues with the current implementation:

  1. Cases like GHSA-h6cc-rc6q-23j4 (+regression fix)
    are not handled correctly
    • The current logic using strings.Fields() splits this into ["GHSA-h6cc-rc6q-23j4", "(+regression", "fix)"]
    • Since len(ids) > 1 and the first ID doesn't
      start with "CVE-", the GHSA ID gets skipped
  2. Comment-only entries like (+regression_fix) would be incorrectly inserted
    • If an entry contains only comments without valid vulnerability IDs, the current logic might try to save invalid data

To address these issues, we need to implement stricter parsing that:

  • Properly extracts valid vulnerability IDs (CVE-, GHSA-, etc.) regardless of additional text
  • Ensures only valid IDs are saved to the database
  • Handles edge cases where no valid IDs are present
diff --git a/pkg/vulnsrc/alpine/alpine.go b/pkg/vulnsrc/alpine/alpine.go
index ec7afec..482c196 100644
--- a/pkg/vulnsrc/alpine/alpine.go
+++ b/pkg/vulnsrc/alpine/alpine.go
@@ -97,24 +97,34 @@ func (vs VulnSrc) saveSecFixes(tx *bolt.Tx, platform, pkgName string, secfixes m
 		for _, vulnID := range vulnIDs {
 			// See https://gitlab.alpinelinux.org/alpine/infra/docker/secdb/-/issues/3
 			// e.g. CVE-2017-2616 (+ regression fix)
+			// e.g. GHSA-h6cc-rc6q-23j4 (+regression fix)
 			ids := strings.Fields(vulnID)
-			for _, cveID := range ids {
-				cveID = strings.ReplaceAll(cveID, "CVE_", "CVE-")
-				// Possible cases when vulnID contains only one non-CVE-ID
-				// e.g. only GHSA-ID
-				// cf. https://github.com/aquasecurity/vuln-list/blob/a830a1cc031f51a29a1a6f1c28d8366bbb5e7b64/alpine/3.21/community/vaultwarden.json#L9-L13
-				if !strings.HasPrefix(cveID, "CVE-") && len(ids) > 1 {
-					continue
-				}
-				if err := vs.dbc.PutAdvisoryDetail(tx, cveID, pkgName, []string{platform}, advisory); err != nil {
-					return oops.Wrapf(err, "failed to save advisory")
-				}
 
-				// for optimization
-				if err := vs.dbc.PutVulnerabilityID(tx, cveID); err != nil {
-					return oops.Wrapf(err, "failed to save the vulnerability ID")
+			// Find CVE-ID first, if not found, use GHSA-ID
+			var selectedID string
+			for _, id := range ids {
+				id = strings.ReplaceAll(id, "CVE_", "CVE-")
+				if strings.HasPrefix(id, "CVE-") {
+					selectedID = id
+					break // CVE-ID found, prioritize it
+				} else if strings.HasPrefix(id, "GHSA-") {
+					selectedID = id // Use GHSA-ID if no CVE-ID found yet
 				}
 			}
+
+			if selectedID == "" {
+				// No valid vulnerability ID found
+				continue
+			}
+
+			if err := vs.dbc.PutAdvisoryDetail(tx, selectedID, pkgName, []string{platform}, advisory); err != nil {
+				return oops.Wrapf(err, "failed to save advisory")
+			}
+
+			// for optimization
+			if err := vs.dbc.PutVulnerabilityID(tx, selectedID); err != nil {
+				return oops.Wrapf(err, "failed to save the vulnerability ID")
+			}
 		}
 	}
 	return nil
diff --git a/pkg/vulnsrc/alpine/alpine_test.go b/pkg/vulnsrc/alpine/alpine_test.go
index 9f75a6d..58b5ab5 100644
--- a/pkg/vulnsrc/alpine/alpine_test.go
+++ b/pkg/vulnsrc/alpine/alpine_test.go
@@ -59,6 +59,12 @@ func TestVulnSrc_Update(t *testing.T) {
 						FixedVersion: "2.9.3-r0",
 					},
 				},
+				{
+					Key: []string{"advisory-detail", "GHSA-h6cc-rc6q-23j4", "alpine 3.12", "ansible"},
+					Value: types.Advisory{
+						FixedVersion: "2.9.3-r0",
+					},
+				},
 				{
 					Key: []string{"advisory-detail", "CVE-2020-1737", "alpine 3.12", "ansible"},
 					Value: types.Advisory{
@@ -100,6 +106,13 @@ func TestVulnSrc_Update(t *testing.T) {
 					},
 					Value: map[string]interface{}{},
 				},
+				{
+					Key: []string{
+						"vulnerability-id",
+						"GHSA-h6cc-rc6q-23j4",
+					},
+					Value: map[string]interface{}{},
+				},
 				{
 					Key: []string{
 						"vulnerability-id",
diff --git a/pkg/vulnsrc/alpine/testdata/happy/vuln-list/alpine/ansible.json b/pkg/vulnsrc/alpine/testdata/happy/vuln-list/alpine/ansible.json
index 7bd87e9..83be130 100644
--- a/pkg/vulnsrc/alpine/testdata/happy/vuln-list/alpine/ansible.json
+++ b/pkg/vulnsrc/alpine/testdata/happy/vuln-list/alpine/ansible.json
@@ -6,7 +6,8 @@
       "CVE-2019-14905",
       "CVE-2024-22195 GHSA-h5c8-rqwp-cp95",
       "CVE-2017-2616 (+ regression fix)",
-      "GHSA-f7r5-w49x-gxm3"
+      "GHSA-f7r5-w49x-gxm3",
+      "GHSA-h6cc-rc6q-23j4 (+regression fix)"
     ],
     "2.9.6-r0": [
       "CVE-2020-1737"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It seems that after your issue Alpine did not add invalid data (like `(+regression fix)) to Vuln-ID fields.
And I hope that will not happen

but there can always be errors, so I implemented your idea anyway - 83c3e19

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

That's good news. But advisories often contain invalid or unreliable data, so it's best to filter out anything that's obviously incorrect.

@knqyf263 knqyf263 merged commit 64cb824 into aquasecurity:main May 28, 2025
1 check passed
@DmitriyLewen DmitriyLewen deleted the fix/alpine/ghsa-id-only branch May 28, 2025 11:26
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 this pull request may close these issues.

2 participants