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

Patch details problems #8

Open
orchestr7 opened this issue Aug 29, 2023 · 7 comments
Open

Patch details problems #8

orchestr7 opened this issue Aug 29, 2023 · 7 comments

Comments

@orchestr7
Copy link
Member

  1. patches_detail[] is now added as a separate field on the same level as affected[]:. It is incorrect, because we cannot make a mapping from an AFFECTED PROJECT (affected[]) to a patch where it was fixed and cannot properly show it.

Imagine, that your library XXX has two major versions 1.0.0 and 3.0.0 (both are supported).
Vulnerability fixes usually go into both version, and in COSV we say:

"ranges": [ {
    "type": "SEMVER",
    "events": [
      { "introduced": "1.0.0" },
      { "fixed": "1.0.2" },
      { "introduced": "3.0.0" },
      { "fixed": "3.2.5" }
    ]
} ]

And patches were different with different commits IDs. In current schema we cannot do anything and create mapping between those project and fix.

Suggestion to move patches_detail to affected[].ranges[] or affected[]. PLease think about that.

  1. affected[].package.language is DUPLICATED with patches_detail[].main_language
@JustinB1eber
Copy link

JustinB1eber commented Aug 30, 2023

Thanks for your careful and considerate. Here's my opinions:

  1. About patches_detail[]. At the beginning of the design, we hope that all data is a direct extension of the vulnerability, and the patch and the affected package use the vulnerability as a bridge to connect. But the problem you said exists.
  • However, as far as the current vulnerability patch data situation is concerned, no vulnerability database provides a mapping between patches and specific fix versions on vulnerability reports. If we want to further correlate the patch to the affected version range based on the existing data, it may take a lot of time and cost.
  • Once an accurate commit link as a patch is provided, with the code repository cloned, users can find out which branch the repair patch is applied to according to the git graph.
  • If we want to associate the patch with the affected version range, we may need to modify the affected[].package.ranges field, and how to continue to be compatible with OSV would be a problem.
  1. About affected[].package.language , it is used to indicate the major language of package is developed in. This may be mainly used to give classification labels when displaying vulnerability reports, but for users who use this package as a dependency, it has no clear effect. They don't care what language the dependent package is developed in. patches_detail[].main_language is a more precise field, and it has more practical use. Most of the time, the repair of a patch involves code logic changes, common in code languages such as Java and C++, and sometimes only involves modification of configuration files, such as xml. For example, for those researchers who use fixing patch as dataset to train vulnerability-related deep learning models, this field would be important.

@orchestr7
Copy link
Member Author

@JustinB1eber but we are not able to match/map the patch to the project now.

Affected[] and patch_details[] are on the same level. If there are multiple records in each of these archives, we would not be able to make mapping.

How do we can do it?

@orchestr7
Copy link
Member Author

Proposal: to add patch_details to events

"events": [
      { "introduced": "0" },
      { "fixed": "1.0.2", "patch_details": "PATCH" },
      { "introduced": "3.0.0" },
      { "fixed": "3.1.2" },

For introduced you can skip it, for fix we will always have possibility to add these details and it will not break OSV schema, as this field will be optional. We will be still able to support OSV and COSV with the same deserialization mechanism

@JustinB1eber

@JustinB1eber
Copy link

@akuleshov7
We don't have to mapping patch_details[] to affected projects by move one into another, because there is a original mapping between patches_detail[].patch_url and affected[].ranges[].repo

@JustinB1eber
Copy link

{ "fixed": "1.0.2", "patch_details": "PATCH" } this might introduce a new problem is that when there is a patch fix in PR and the new version of hotfix is not being released we don't know where to put this patches_detail[].
For example in linux kernel, there are a lot of downstream OS projects forked from it, when those developers first time get the patch with commit ID, they cherry-pick it onto their branches and keep on developing, not to wait for the release.

@orchestr7
Copy link
Member Author

{ "fixed": "1.0.2", "patch_details": "PATCH" } this might introduce a new problem is that when there is a patch fix in PR and the new version of hotfix is not being released we don't know where to put this patches_detail[]. For example in linux kernel, there are a lot of downstream OS projects forked from it, when those developers first time get the patch with commit ID, they cherry-pick it onto their branches and keep on developing, not to wait for the release.

It's OPTIONAL. You don't need to put patch details to event.

@orchestr7
Copy link
Member Author

As discussed with @nulls events should also be extended by some time field (OPTIONAL):
{ "fixed": "1.0.2", "patch_details": "PATCH" , "time": "TIME" }

With this change we will cover cases with several fixes in different versions and packages.
So we can remove several timeline events: found, fixed and introduced (calculatable from affected[]).

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

No branches or pull requests

2 participants