Suggest improvements for this vulnerability feature doesn't support > operator

[mrjonstrong]

I went to this advisory GHSA-8489-44mv-ggj8 and on the right clicked the Suggest improvements for this vulnerability.

I just removed the log4j-api packages and left the rest as they were. Clicked "Submit Improvements" and had this error that the > operator is not supported.

[chrisbloom7]

@mrjonstrong, I'm sorry that you bumped up against this in the first 24-hours of this feature going live. There are currently about 3 dozen advisories out of ~7000 that could not be exported to this advisory database because of that > operator which is incompatible with the OSV schema, but which is sometimes necessary due to limitations in our current internal reporting process. This advisory happens to be one of them. We're working with our curation team to clean up as many of those as possible. Unfortunately in the interim this means community members cannot make suggestions on this handful of affected advisories because they are not present in this repo, i.e. even if your update removed all of the > operators, the update would still fail because there's no file to commit a change against. We do not have a workaround at present, but we're aware it's an issue and we're actively tracking it.

[mrjonstrong]

@chrisbloom7 No worries at all, happy you got this feature live, it's a great addition!
Thanks for the explanation, glad you are on it 👍
It's OK I can just do a manual pull request still.
Does it help to leave this issue open in case anyone else comes looking for something similar, help to track etc? Or feel free to close it if it won't add any benefit.

[chrisbloom7]

You're welcome!

It's OK I can just do a manual pull request still.

Unfortunately this won't work because you won't find a GHSA-8489-44mv-ggj8.json file in the repo to make changes to, for the same incompatibility issue noted above. I can follow up with the curation team to see if they want to manually incorporate the change you were trying to make.

I think we can leave this issue open for now until we have a better solution, in case, as you say, someone else runs into this.

[G-Rath]

@chrisbloom7 could you clarify how the OSV schema is not compatible with >? (I'm guessing the issue is that we don't know what the next version is, since you could have a build version with an arbitrary value 🤔)

I've built osv-detector which uses this database, and have come across this issue while doing some manual smoke testing by comparing to the audit results of other tools.

So far, I've come across GHSA-93q8-gq69-wqmw being missing - I would expect you should fix this advisory by changing it to be >= 2.1.2 instead of > 2.1.1.

I'm hoping to be able to switch over to using osv-detector for doing our auditing at work (instead of having to manage all the different package managers & tools), but this represents something of a blocker - let me know if I can help in anyway.

Here's a complete list of all the >s I've come across:

• GHSA-93q8-gq69-wqmw
• GHSA-jxhc-q857-3j6g

[chrisbloom7]

@G-Rath We're aware of about 3 dozen vulnerabilities that are currently using the > operator in their version ranges. The OSV Schema currently (as of v1.2) has no way of representing a lower bound exclusive version directly. There was a brief discussion about adding support for it via a new event type or some new field, but I eventually closed the issue because it was clear after talking it through that our use of it came down to an internal dependency based on our curation tooling and not some industry standard that should be widely adopted. While it might seem safe to assume that >= 2.1.2 is equivalent to > 2.1.1 this is not always the case, and making that determination often requires interfacing with a package manager for the given ecosystem and/or some human intervention to read through vulnerability reports. We do not currently have a way to communicate with package managers for all supported ecosystems when we export our advisory data to OSV JSON, so when we do run into the rare vulnerable version range that includes the > operator we simply have to skip converting the advisory. The reasons why a > may appear in advisories that we are curating comes down to a call by our curators based on whatever they feel best describes the vulnerable range, even if it means the advisory may not be immediately exportable to this repo. We continue to refine both curation tooling and workflows so hopefully the > operator continues to be a rare exception, and advisories that use it can be updated to use other operators when possible.

[G-Rath]

@chrisbloom7 thanks for that write-up and link - that was the reason I was thinking. While I understand wanting to be as accurate as possible, it does feel like skipping the advisory all together results in a net loss, since if I'm understanding correctly the primary reason why your team is using > is due to lack of manual resources to do the digging e.g. like that was done here for every single advisory (which is understandable, as that'd be a lot of work), but then by not having the advisory in this database we the community cannot help improve the advisories for you...

At the same time, it also already looks like you have a possible mechanisms to helping support this: some advisories in this database have an last_known_affected_version_range, e.g. GHSA-mg2g-8pwj-r2j2.

None of this is meant as a negative, but I find it a bit uneasy to know that there are a handful of advisories we have no insight into at all right now which means we can't really account for them so am very interested in helping getting this improved if possible.

(I mean, I'd be happier if you just had another folder named unconvertable that you stuck these advisories into in a as-close-as-possible form; that way we'd be able to at least see them and could try to manually include them in our tools)

[ljharb]

Couldn't you convert > 1.2.3 into >= 1.2.3 and not 1.2.3 somehow? That wouldn't involve the assumption that 1.2.3 and 1.2.4 have no versions between them, but it would avoid missing out on these advisories.

[chrisbloom7]

@G-Rath good eye! last_known_affected_version_range is a hinting field that we can use internally to rehydrate certain range operators from OSV back to our internal representation. This works for upper bound inclusive ranges, but doesn't work for lower bound exclusive because, again, they simply aren't unrepresentable in OSV so there is nothing to attach a hint to. (Basically it would have to be an OSV file with no version info aside from our database-specific hints, which aren't intended to be actionable outside of our publication system and therefore not useful)

@ljharb Yes, that would definitely be preferable in some cases and is how we're addressing some of those 3 dozen advisories that we've flagged as currently unpublishable. That said, our curation team takes a lot of information into account and we need to provide them the flexibility to use the > operator when they feel it's necessary since the info they provide feeds multiple downstream systems, not just this repo. This is one aspect of the curation workflow we hope to be able to improve in the future.

[chrisbloom7]

None of this is meant as a negative, but I find it a bit uneasy to know that there are a handful of advisories we have no insight into at all right now which means we can't really account for them so am very interested in helping getting this improved if possible.

@G-Rath, just to be clear, these advisories are still available in github.com/advisories and via our GraphQL advisory API. They just aren't representable as OSV JSON and thus don't appear in this database mirror. We understand that's less than ideal and we have had an uncountable number of conversations about the > operator and it's impact on this project. 😆 😭

[G-Rath]

@chrisbloom7 but then I'll have to talk to that API which is sort of the opposite of what osv-detector is meant to be doing - plus, I can't query for just advisories affected by this problem so right now it's a sort of all-or-nothing where if I use the GraphQL API I've got to deal with requests, rate limiting, always-online, etc or if I use this database in which case I have to be concerned about being affected by known security advisories that are not in this database.

I mean, even just having a text file listing the IDs of all the affected advisories maintained in this repo would improve this for me short-term, since we could use that to query the API directly for just those advisories.

[chrisbloom7]

@G-Rath I hear what you are saying and understand why it's problematic for not just your use case but for many others as well.

I mean, even just having a text file listing the IDs of all the affected advisories maintained in this repo would improve this for me short-term, since we could use that to query the API directly for just those advisories.

That could be an interesting addition, like a table of contents. I'll add a note about your suggestion to the tracking issue for this.

[KateCatlin]

Hello all! We've resolved this issue and replaced the > operator with inclusive versioning in our advisories. Our Curation team has been trained to no longer use this in the future.

There is one very specific edge case which is still unable to be translated to OSV, but since that case does not relate to the discussion here we'll consider it separately.

Thanks for chiming in here and providing us feedback along the way!