Skip to content
This repository has been archived by the owner on Mar 8, 2023. It is now read-only.

Assume a link to be a "patch link" only if tagged as Patch by NVD #44

Closed
sbs2001 opened this issue Sep 24, 2020 · 6 comments · Fixed by #45
Closed

Assume a link to be a "patch link" only if tagged as Patch by NVD #44

sbs2001 opened this issue Sep 24, 2020 · 6 comments · Fixed by #45
Assignees
Labels
enhancement New feature or request p2

Comments

@sbs2001
Copy link
Contributor

sbs2001 commented Sep 24, 2020

Currently VulnCode-DB assumes every link which passes the regex

PATCH_REGEX = r".*(github\.com|\.git|\.patch|\/hg\.|\/\+\/)"
at
def get_patches(self):

as a patch link. In most cases this is correct but there are cases when this fails to identify patch links correctly such as at

https://www.vulncode-db.com/CVE-2018-7749 the patch link is identified as ronf/asyncssh@c161e26 . But this link does not contain any example of "vulnerable code".

Such type of error(s) can be avoided if we add one more condition for a link to be considered as a patch, this condition would be to check whether NVD tags the reference/link as a Patch . For CVE-2018-7749 NVD has rightly tagged it as a Third Party Advisory NOT Patch at https://nvd.nist.gov/vuln/detail/CVE-2018-7749 .

If such errors don't occur at considerable frequency close this ticket.

@evonide
Copy link
Contributor

evonide commented Sep 27, 2020

Thanks that's good to know! Unfortunately, the dependency https://github.com/kotakanbe/go-cve-dictionary/ we use to fetch and incorporate the NVD data seems to be unaware of Reference tags such as Third Party Advisory.

Particularly, looking at: https://github.com/kotakanbe/go-cve-dictionary/blob/master/models/models.go#L257

// Reference is Child model of Jvn/Nvd.
// It holds reference information about the CVE.
type Reference struct {
	gorm.Model `json:"-" xml:"-"`
	NvdJSONID  uint `json:"-" xml:"-"`
	JvnID      uint `json:"-" xml:"-"`

	Source string
	Link   string `sql:"type:text"`
}

You can see that the column for tags is missing. I think this is processed in https://github.com/kotakanbe/go-cve-dictionary/blob/master/fetcher/nvd/json/nvd.go#L253
tag := strings.Join(ref.TAGS, " ")
but not stored anywhere.

One approach would be to provide a pull request to modify the data models and to store the tags data seperated by ',' as is the case in the NVD provided JSON format (https://nvd.nist.gov/feeds/json/cve/1.1/nvdcve-1.1-recent.json.zip), too.

@sbs2001
Copy link
Contributor Author

sbs2001 commented Sep 27, 2020

@evonide Thanks for the research.

For the PR solution to work, #41 should be get done with.

A quick fix would be to modify vulncode-db's go-cve-dictionary fork . With some minor refactors propagating our changes to latest upstream will be easy(and then file a PR there) .

If this works for you please assign me this ticket (and expect a PR soon :) ) .

@evonide evonide added enhancement New feature or request p2 labels Sep 27, 2020
@evonide
Copy link
Contributor

evonide commented Sep 27, 2020

Sounds good I'll get back once I've looked into resolving #41.

@evonide
Copy link
Contributor

evonide commented Oct 3, 2020

Hey Shivam, as promised I've updated go-cve-dictionary to the latest state in #41.
Assigning to you as suggested. Please let me know if I can help with this.

@sbs2001
Copy link
Contributor Author

sbs2001 commented Oct 5, 2020

@evonide please review #45 .

Unrelated to this, but I was wondering why are their migrations for the cve database at 2 places ? There's migrations at alembic and also go-cve-dictionary creates the same schema. This creates issues when go-cve-dictionary is ran before frontend. The alembic migrations then attempt to create the same schema which was created before by go-cve-dictionary.

@evonide
Copy link
Contributor

evonide commented Oct 9, 2020

Thanks and fully agreed Shivam! I'll look into how to best resolve this soon.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants