Migrate to VulnerableCode API v3#11961
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| * Retrieve all advisories affecting the given [purl]. This is done by paging through the results of the respective | ||
| * endpoint until there are no more pages left. | ||
| */ | ||
| private suspend fun HttpClient.getAllAffectedByAdvisories(purl: String): List<AffectedByAdvisoryV3> { |
| ): List<VulnerabilityReference> = | ||
| runCatching { | ||
| val sourceUri = URI(url.fixupUrlEscaping()) | ||
| private fun AffectedByAdvisoryV3.toReferences(issues: MutableList<Issue>): List<VulnerabilityReference> { |
| * Convert this advisory reference from the VulnerableCode data model to a [VulnerabilityReference] object. | ||
| * Populate [issues] if this fails. | ||
| */ | ||
| private fun AdvisoryReference.toModel(issues: MutableList<Issue>): VulnerabilityReference? = |
| * Convert this advisory severity from the VulnerableCode data model to a [VulnerabilityReference] object. | ||
| * Populate [issues] if this fails. | ||
| */ | ||
| private fun AdvisorySeverity.toModel(fallbackUrl: String, issues: MutableList<Issue>): VulnerabilityReference? { |
| */ | ||
| private fun VulnerableCodeService.Vulnerability.preferredCommonId(): String { | ||
| if (aliases.isEmpty()) return vulnerabilityId | ||
| private fun AffectedByAdvisoryV3.preferredCommonId(): String { |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #11961 +/- ##
=========================================
Coverage 58.44% 58.44%
Complexity 1811 1811
=========================================
Files 361 361
Lines 13504 13504
Branches 1385 1385
=========================================
Hits 7893 7893
Misses 5115 5115
Partials 496 496
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
lamppu
force-pushed
the
11050-vulnerablecode-api-v3
branch
3 times, most recently
from
4369687 to
820fa06
Compare
Add the public VulnerableCode OpenAPI schema to the client module and configure kfx to auto-generate Kotlin sources from it. Signed-off-by: Johanna Lamppu <johanna.lamppu@doubleopen.io>
Patch the VulnerableCode OpenAPI schema on the missing parts for the `v3/packages` and `v3/affected-by-advisories` endpoints. Use `"type": "string"` for the `scoring_system` field to work around an issue with the generated enums [1]. Also remove some elements from the `required` arrays to work around an issue with generated code regarding nullable fields [2]. [1]: hfhbd/kfx#259 [2]: hfhbd/kfx#260 Signed-off-by: Johanna Lamppu <johanna.lamppu@doubleopen.io>
Use `v3/packages` bulk search to filter out PURLs that don't have results, and gather vulnerability details for each affected PURL from the `v3/affected-by-advisories` results. `v3/packages` is called with the `details=false` option, because the additional details - returned with `details=true` - do not contain the data needed for the references, and so the `v3/affected-by-advisories` needs to be called separately for each PURL*. `v3/packages` also returns PURLs for packages that fix vulnerabilities, so these are later dropped if there are no advisories found. The v3 model doesn't provide scores for the individual references anymore, only the `severities` field from the advisory results contain this information, so gather the references from both the `severities` and `references` fields. * There is a `v3/advisories` bulk search but it doesn't provide information on which PURLs are affected by the listed advisories. Resolves oss-review-toolkit#11050. Signed-off-by: Johanna Lamppu <johanna.lamppu@doubleopen.io>
lamppu
force-pushed
the
11050-vulnerablecode-api-v3
branch
from
820fa06 to
f1d300e
Compare
| kotlinpoet-ksp = { module = "com.squareup:kotlinpoet-ksp", version.ref = "kotlinPoet"} | ||
| kotlinx-coroutines = { module = "org.jetbrains.kotlinx:kotlinx-coroutines-core", version.ref = "kotlinxCoroutines" } | ||
| kotlinx-html = { module = "org.jetbrains.kotlinx:kotlinx-html-jvm", version.ref = "kotlinxHtml" } | ||
| kotlinx-datetime = { module = "org.jetbrains.kotlinx:kotlinx-datetime", version.ref = "kotlinxDatetime" } |
There was a problem hiding this comment.
Nit: This line should go one up.
| ], | ||
| "responses": { | ||
| "201": { | ||
| "200": { |
There was a problem hiding this comment.
Please report the schema issues upstream and add a link to the issue to the first paragraph of the commit message.
|
|
||
| "Vulnerable Go packages" should { | ||
| "return findings for QUIC" { | ||
| // VulnerableCode v3 API doesn't currently have results for this package. |
There was a problem hiding this comment.
This is also something that should be reported upstream. It should not depend on the version of the API used whether a package is vulnerable or not.
| server.stubPackagesRequest("packages_response_packages.json") | ||
| server.stubAffectedByAdvisoriesRequest(idLang, "affected_by_advisories_response_lang.json") | ||
| server.stubAffectedByAdvisoriesRequest(idStruts, "affected_by_advisories_response_struts.json") | ||
|
|
There was a problem hiding this comment.
In most other tests you omit the empty line here (i guess, to more strictly follow the AAA pattern), so you should omit it here, too.
| server.stubPackagesRequest("response_junit.json", request = generatePackagesRequest(idJUnit)) | ||
| server.stubPackagesRequest("packages_response_junit.json", request = generatePackagesRequest(idJUnit)) | ||
| server.stubAffectedByAdvisoriesRequest(idJUnit, "affected_by_advisories_response_junit.json") | ||
|
|
| scoringSystem should beNull() | ||
| severity should beNull() | ||
| score should beNull() | ||
| vector should beNull() |
There was a problem hiding this comment.
This is unexpected... any idea why other references have data here, but this reference not? I probably need to debug to see for myself whether the better adaption would be to change the reference URL instead of the expected values.
There was a problem hiding this comment.
So the pattern is approximately this, two advisories are returned for each vulnerability, one GitHub advisory and one GitLab advisory. The GitHub advisory always seems to contain two severities and the GitLab advisory one severity. The severities are the objects that contain scoring data. Both advisories also have several references, but these only contain url, reference_type and reference_id fields. So they don't have any scoring data available. I'm currently collecting VulnerabilityReference objects from both the severities and the references.
As an example, here are the advisories returned for this "CVE-2023-2976" vulnerability for the Guava package:
{
"advisory_id": "github_osv_importer_v2/GHSA-7g45-4rm6-3mm3",
"url": "https://github.com/github/advisory-database/blob/main/advisories/github-reviewed/2023/06/GHSA-7g45-4rm6-3mm3/GHSA-7g45-4rm6-3mm3.json",
"aliases": [
"CVE-2023-2976"
],
"summary": "Guava vulnerable to insecure use of temporary directory\nUse of Java's default temporary directory for file creation in `FileBackedOutputStream` in Google Guava versions 1.0 to 31.1 on Unix systems and Android Ice Cream Sandwich allows other users and apps on the machine with access to the default Java temporary directory to be able to access the files created by the class.\n\nEven though the security vulnerability is fixed in version 32.0.0, maintainers recommend using version 32.0.1 as version 32.0.0 breaks some functionality under Windows.",
"severities": [
{
"url": "https://github.com/github/advisory-database/blob/main/advisories/github-reviewed/2023/06/GHSA-7g45-4rm6-3mm3/GHSA-7g45-4rm6-3mm3.json",
"value": "5.5",
"scoring_system": "cvssv3.1",
"scoring_elements": "CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:U/C:H/I:N/A:N"
},
{
"url": null,
"value": "MODERATE",
"scoring_system": "generic_textual",
"scoring_elements": ""
}
],
"weaknesses": [
{
"cwe_id": "552",
"name": "Files or Directories Accessible to External Parties",
"description": "The product makes files or directories accessible to unauthorized actors, even though they should not be."
},
{
"cwe_id": "379",
"name": "Creation of Temporary File in Directory with Insecure Permissions",
"description": "The product creates a temporary file in a directory whose permissions allow unintended actors to determine the file's existence or otherwise access that file."
}
],
"references": [
{
"url": "https://github.com/google/guava",
"reference_type": "",
"reference_id": ""
},
{
"url": "https://github.com/google/guava/commit/feb83a1c8fd2e7670b244d5afd23cba5aca43284",
"reference_type": "",
"reference_id": ""
},
{
"url": "https://github.com/google/guava/issues/2575",
"reference_type": "",
"reference_id": ""
},
{
"url": "https://github.com/google/guava/issues/6532",
"reference_type": "",
"reference_id": ""
},
{
"url": "https://github.com/google/guava/releases/tag/v32.0.0",
"reference_type": "",
"reference_id": ""
},
{
"url": "https://security.netapp.com/advisory/ntap-20230818-0008",
"reference_type": "",
"reference_id": ""
},
{
"url": "https://security.netapp.com/advisory/ntap-20241108-0002",
"reference_type": "",
"reference_id": ""
},
{
"url": "https://www.intel.com/content/www/us/en/security-center/advisory/intel-sa-01006.html",
"reference_type": "",
"reference_id": ""
},
{
"url": "https://nvd.nist.gov/vuln/detail/CVE-2023-2976",
"reference_type": "",
"reference_id": "CVE-2023-2976"
}
],
"exploitability": "0.5",
"weighted_severity": "5.0",
"risk_score": "2.5",
"related_ssvc_trees": [
{
"vector": "SSVCv2/E:N/A:N/T:T/P:M/B:A/M:M/D:T/2024-04-18T04:00:21Z/",
"decision": "Track",
"options": [
{
"Exploitation": "none"
},
{
"Automatable": "no"
},
{
"Technical Impact": "total"
},
{
"Mission Prevalence": "minimal"
},
{
"Public Well-being Impact": "material"
},
{
"Mission & Well-being": "medium"
}
],
"source_url": "https://github.com/cisagov/vulnrichment/blob/develop/2023/2xxx/CVE-2023-2976.json"
},
{
"vector": "SSVCv2/E:N/A:N/T:T/P:M/B:A/M:M/D:T/2024-04-18T04:00:21Z/",
"decision": "Track",
"options": [
{
"Exploitation": "none"
},
{
"Automatable": "no"
},
{
"Technical Impact": "total"
},
{
"Mission Prevalence": "minimal"
},
{
"Public Well-being Impact": "material"
},
{
"Mission & Well-being": "medium"
}
],
"source_url": "https://github.com/cisagov/vulnrichment/blob/develop/2023/2xxx/CVE-2023-2976.json"
}
],
"fixed_by_packages": [
"pkg:maven/com.google.guava/guava@32.0.0-android"
]
},
{
"advisory_id": "gitlab_importer_v2/maven/com.google.guava/guava/CVE-2023-2976",
"url": "https://gitlab.com/gitlab-org/advisories-community/-/blob/main/maven/com.google.guava/guava/CVE-2023-2976.yml",
"aliases": [
"CVE-2023-2976",
"GHSA-7g45-4rm6-3mm3"
],
"summary": "Guava vulnerable to insecure use of temporary directory\nUse of Java's default temporary directory for file creation in `FileBackedOutputStream` in Google Guava versions 1.0 to 31.1 on Unix systems and Android Ice Cream Sandwich allows other users and apps on the machine with access to the default Java temporary directory to be able to access the files created by the class.\n\nEven though the security vulnerability is fixed in version 32.0.0, maintainers recommend using version 32.0.1 as version 32.0.0 breaks some functionality under Windows.",
"severities": [
{
"url": "https://gitlab.com/gitlab-org/advisories-community/-/blob/main/maven/com.google.guava/guava/CVE-2023-2976.yml",
"value": "None",
"scoring_system": "cvssv3.1",
"scoring_elements": "CVSS:3.1/AV:L/AC:L/PR:L/UI:N/S:U/C:H/I:N/A:N"
}
],
"weaknesses": [
{
"cwe_id": "552",
"name": "Files or Directories Accessible to External Parties",
"description": "The product makes files or directories accessible to unauthorized actors, even though they should not be."
},
{
"cwe_id": "379",
"name": "Creation of Temporary File in Directory with Insecure Permissions",
"description": "The product creates a temporary file in a directory whose permissions allow unintended actors to determine the file's existence or otherwise access that file."
},
{
"cwe_id": "937",
"name": "OWASP Top Ten 2013 Category A9 - Using Components with Known Vulnerabilities",
"description": "Weaknesses in this category are related to the A9 category in the OWASP Top Ten 2013."
},
{
"cwe_id": "1035",
"name": "OWASP Top Ten 2017 Category A9 - Using Components with Known Vulnerabilities",
"description": "Weaknesses in this category are related to the A9 category in the OWASP Top Ten 2017."
}
],
"references": [
{
"url": "https://github.com/google/guava",
"reference_type": "",
"reference_id": ""
},
{
"url": "https://github.com/google/guava/commit/feb83a1c8fd2e7670b244d5afd23cba5aca43284",
"reference_type": "",
"reference_id": ""
},
{
"url": "https://github.com/google/guava/issues/2575",
"reference_type": "",
"reference_id": ""
},
{
"url": "https://github.com/google/guava/issues/6532",
"reference_type": "",
"reference_id": ""
},
{
"url": "https://github.com/google/guava/releases/tag/v32.0.0",
"reference_type": "",
"reference_id": ""
},
{
"url": "https://security.netapp.com/advisory/ntap-20230818-0008",
"reference_type": "",
"reference_id": ""
},
{
"url": "https://security.netapp.com/advisory/ntap-20241108-0002",
"reference_type": "",
"reference_id": ""
},
{
"url": "https://www.intel.com/content/www/us/en/security-center/advisory/intel-sa-01006.html",
"reference_type": "",
"reference_id": ""
},
{
"url": "https://nvd.nist.gov/vuln/detail/CVE-2023-2976",
"reference_type": "",
"reference_id": "CVE-2023-2976"
},
{
"url": "https://github.com/advisories/GHSA-7g45-4rm6-3mm3",
"reference_type": "",
"reference_id": "GHSA-7g45-4rm6-3mm3"
}
],
"exploitability": "0.5",
"weighted_severity": "0.0",
"risk_score": null,
"related_ssvc_trees": [
{
"vector": "SSVCv2/E:N/A:N/T:T/P:M/B:A/M:M/D:T/2024-04-18T04:00:21Z/",
"decision": "Track",
"options": [
{
"Exploitation": "none"
},
{
"Automatable": "no"
},
{
"Technical Impact": "total"
},
{
"Mission Prevalence": "minimal"
},
{
"Public Well-being Impact": "material"
},
{
"Mission & Well-being": "medium"
}
],
"source_url": "https://github.com/cisagov/vulnrichment/blob/develop/2023/2xxx/CVE-2023-2976.json"
}
],
"fixed_by_packages": [
"pkg:maven/com.google.guava/guava@32.0.0-android"
]
}
|
This will break compatibility with VulnerableCode API V1 and V2, correct? |
It will remove API v1 support (API v2 was never implemented), that's correct. But to what extend is that a (user-facing) breaking change according to our rules? From an ORT user perspective, everything should just work as before, i.e. no URL or API key changes needed (letting aside the fact that we now anyway need to use API keys due to general changes in VulnerabileCode rate limiting, independently of the API). @lamppu, please correct me if I'm wrong. Are you after the code changes to the |
Users that use an on-premises version of VulnerableCode (like Bosch) are forced to upgrade to VulnerableCode v38+, which is not a trivial task with the changes in v37 and v38. Removing the support for API V1 has therefore quite severe implications on these ORT users. Maybe according to that rule it might be also a breaking change according to the ORT rules:
But that is debatable. I know that we have to support API V3 soon, but we have to be aware of the implications (of removing V1 support). |
Ah, I wasn't aware that you're running a version that does not even support API v3. So, should we aim for supporting both APIs in parallel, and either create a new advisor, or make the existing advisor configurable WRT which API version to use? |
It would be great to keep the support for the older API at least for a certain grace period. We are trying to upgrade VC as soon as possible, but are currently facing massive problems with an increased database load. It will probably take us some time before we solve this. |
Ok, so would the preferable pattern be to introduce a config option for |
|
@lamppu, let's discuss in the community meeting tomorrow. If a goal is to be able to compare results from API v1 and v3, then I believe we need separate advisors, as we cannot have multiple instances of the same advisor with different options. |
5 participants
Please see the individual commits for details.