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

fix(cyclonedx): fix work when there are same pkgs from different dirs #5798

Conversation

DmitriyLewen
Copy link
Contributor

@DmitriyLewen DmitriyLewen commented Dec 18, 2023

Description

Fixes for cases when same packages are found in different directories:

➜ tree dir
dir
├── dir1
│   └── jackson-databind-2.13.4.jar
└── dir2
    └── jackson-databind-2.13.4.jar
  1. return all found packages:
    before:
  "components": [
    {
      "bom-ref": "pkg:maven/com.fasterxml.jackson.core/[email protected]?file_path=dir1%2Fjackson-databind-2.13.4.jar",
      "type": "library",
      "group": "com.fasterxml.jackson.core",
      "name": "jackson-databind",
      "version": "2.13.4",
      "purl": "pkg:maven/com.fasterxml.jackson.core/[email protected]",
      "properties": [
        {
          "name": "aquasecurity:trivy:FilePath",
          "value": "dir1/jackson-databind-2.13.4.jar"
        },
        {
          "name": "aquasecurity:trivy:PkgType",
          "value": "jar"
        }
      ]
    }
  ],

after:

  "components": [
    {
      "bom-ref": "pkg:maven/com.fasterxml.jackson.core/[email protected]?file_path=dir1%2Fjackson-databind-2.13.4.jar",
      "type": "library",
      "group": "com.fasterxml.jackson.core",
      "name": "jackson-databind",
      "version": "2.13.4",
      "purl": "pkg:maven/com.fasterxml.jackson.core/[email protected]",
      "properties": [
        {
          "name": "aquasecurity:trivy:FilePath",
          "value": "dir1/jackson-databind-2.13.4.jar"
        },
        {
          "name": "aquasecurity:trivy:PkgType",
          "value": "jar"
        }
      ]
    },
    {
      "bom-ref": "pkg:maven/com.fasterxml.jackson.core/[email protected]?file_path=dir2%2Fjackson-databind-2.13.4.jar",
      "type": "library",
      "group": "com.fasterxml.jackson.core",
      "name": "jackson-databind",
      "version": "2.13.4",
      "purl": "pkg:maven/com.fasterxml.jackson.core/[email protected]",
      "properties": [
        {
          "name": "aquasecurity:trivy:FilePath",
          "value": "dir2/jackson-databind-2.13.4.jar"
        },
        {
          "name": "aquasecurity:trivy:PkgType",
          "value": "jar"
        }
      ]
    }
  ],
  1. show correct ref's for vulnerabilities.affects.
    before(dir2 and dir2):
      "affects": [
        {
          "ref": "pkg:maven/com.fasterxml.jackson.core/[email protected]?file_path=dir2%2Fjackson-databind-2.13.4.jar",
          "versions": [
            {
              "version": "2.13.4",
              "status": "affected"
            }
          ]
        },
        {
          "ref": "pkg:maven/com.fasterxml.jackson.core/[email protected]?file_path=dir2%2Fjackson-databind-2.13.4.jar",
          "versions": [
            {
              "version": "2.13.4",
              "status": "affected"
            }
          ]
        }
      ]

after(dir1 + dir2):

      "affects": [
        {
          "ref": "pkg:maven/com.fasterxml.jackson.core/[email protected]?file_path=dir1%2Fjackson-databind-2.13.4.jar",
          "versions": [
            {
              "version": "2.13.4",
              "status": "affected"
            }
          ]
        },
        {
          "ref": "pkg:maven/com.fasterxml.jackson.core/[email protected]?file_path=dir2%2Fjackson-databind-2.13.4.jar",
          "versions": [
            {
              "version": "2.13.4",
              "status": "affected"
            }
          ]
        }
      ]

Related issues

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've followed the conventions in the PR title.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

@DmitriyLewen DmitriyLewen self-assigned this Dec 18, 2023
Copy link
Contributor Author

@DmitriyLewen DmitriyLewen left a comment

Choose a reason for hiding this comment

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

I only separated same packages from different directories.
But looks like we can merge these packages into 1 component with multiple aquasecurity:trivy:FilePath properties.
e.g.:

"components": [
    {
      "bom-ref": "pkg:maven/com.fasterxml.jackson.core/[email protected]?file_path=dir1%2Fjackson-databind-2.13.4.jar",
      "type": "library",
      "group": "com.fasterxml.jackson.core",
      "name": "jackson-databind",
      "version": "2.13.4",
      "purl": "pkg:maven/com.fasterxml.jackson.core/[email protected]",
      "properties": [
        {
          "name": "aquasecurity:trivy:FilePath",
          "value": "dir1/jackson-databind-2.13.4.jar"
        },
        {
          "name": "aquasecurity:trivy:FilePath",
          "value": "dir2/jackson-databind-2.13.4.jar"
        },
        {
          "name": "aquasecurity:trivy:PkgType",
          "value": "jar"
        }
      ]
    }
  ],

CycloneDX supports duplicate keys for properties: Unlike key-value stores, properties support duplicate names, each potentially having different values
https://cyclonedx.org/docs/1.5/json/#components_items_properties

@knqyf263 wdyt?

})

// Create package map
pkgs := lo.SliceToMap(result.Packages, func(pkg ftypes.Package) (string, Package) {
pkgID := lo.Ternary(pkg.ID == "", fmt.Sprintf("%s@%s", pkg.Name, utils.FormatVersion(pkg)), pkg.ID)
// To avoid skip same packages with different paths
if pkg.FilePath != "" {
Copy link
Contributor Author

@DmitriyLewen DmitriyLewen Dec 18, 2023

Choose a reason for hiding this comment

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

This only applies to Packages (jar, package.json, etc.) because pkg.FilePath is only saved for Packages:

// Each package metadata have the file path, while the package from lock files does not have.
FilePath string `json:",omitempty"`

Copy link
Contributor Author

@DmitriyLewen DmitriyLewen Dec 18, 2023

Choose a reason for hiding this comment

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

pkgs map is used only for dependency iteration.
But Packages don't contain dependencies (DependsOn fields).

So it doesn't break logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about saving slice of ftypes.Package's or slice of FilePath's, but i understood that we still can't get correct package, because DependsOn doesn't currently contain info about filepath.

Looks like this update is enough for now.

If we will add Package with DependsOn - we will need to update DependsOn struct (and logic for this) and we can update logic for CycloneDX.

@@ -144,12 +144,20 @@ func (e *Marshaler) marshalPackages(metadata types.Metadata, result types.Result

// Group vulnerabilities by package ID
vulns := lo.GroupBy(result.Vulnerabilities, func(v types.DetectedVulnerability) string {
return lo.Ternary(v.PkgID == "", fmt.Sprintf("%s@%s", v.PkgName, v.InstalledVersion), v.PkgID)
pkgID := lo.Ternary(v.PkgID == "", fmt.Sprintf("%s@%s", v.PkgName, v.InstalledVersion), v.PkgID)
if v.PkgPath != "" {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

same:

vulns[i].PkgPath = lib.FilePath

@DmitriyLewen DmitriyLewen marked this pull request as ready for review December 19, 2023 05:31
Copy link

This PR is stale because it has been labeled with inactivity.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and will be auto-closed. label Feb 18, 2024
@DmitriyLewen DmitriyLewen removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and will be auto-closed. label Feb 19, 2024
Copy link

This PR is stale because it has been labeled with inactivity.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and will be auto-closed. label Apr 20, 2024
@topiga
Copy link

topiga commented Apr 25, 2024

This should be reviewed...

@github-actions github-actions bot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and will be auto-closed. label Apr 26, 2024
@DmitriyLewen
Copy link
Contributor Author

DmitriyLewen commented Apr 26, 2024

#6240 fixed this problem.

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.

bug(cyclonedx): Trivy image scan reports and counts the same CVE for the same package multiple times
2 participants