-
-
Notifications
You must be signed in to change notification settings - Fork 607
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
openssl - ubuntu package - versions not matched correctly #1374
Comments
The complete fix, with unit test, is on my fork in a branch already: I can create a merge request if wanted. |
…with upstream DependencyVersion class from the OWASP Dependency-Check project. Signed-off-by: Steve Springett <[email protected]>
Thanks for the investigation work. Likely there will be many more cases, both debian and other distros, that will need to be accounted for over time. However, I think this is a good start. I've incorporated your proposed change along with syncing with the upstream class it was originally derived from. |
Hi, I came to very similar issue related to SBom uploaded ubuntu docker image with openssl inside. Component details:
Dependency Track reports following vulnerability: https://nvd.nist.gov/vuln/detail/CVE-2021-3711
However Ubuntu has this issue already fixed: https://ubuntu.com/security/CVE-2021-3711
It seems, that in Ubuntu LTS version pattern the vulnerability will be forever reported as existing (unless Ubuntu uses full new version of openssl higher than 1.1.1l). Or must be suppressed manually. Is there any way, how to solve this systematically? Probably some feature for importing also some CVE/CPE lists for e.g. Ubuntu? Thanks. |
Theoretically, if DT supported Ubuntu Security Noticies (USN) directly, rather than relying on CPE, this would be solvable. Ubuntu provides an OVAL feed, which is designed to assess a system. However, the SBOM would already have the package information defined in it, so I think using their OVAL feed to evaluate Ubuntu packages should work. https://security-metadata.canonical.com/oval/ RedHat also provides OVAL along with CSRF (possibly CSAF 2.0 in the future), so adding proper support for security advisories specific to Linux distros along with information about the vulnerable packages could work using only the coordinates and/or purl defined in the bom. |
Hi @stevespringett, thanks for comment. Your idea sounds well, I had something like this in mind. Maybe it is more standalone enhancement of feature rather than bug. Thanks for considering this. |
Can we make this also work with Debian (as opposed to just Ubuntu) packages? For example, in the case of A very naive option might be to generalize the above regex to something like |
Good to find this issue, which also affects me. It's a show stopper for me auditing container images based on Debian and Alpine. Even correctly detected versions result in false positives, e.g. openssl 1.1.1n-0+deb11u3 has backported fixes for most CVEs which affect 1.1.1n, but dtrack lists them as unfixed vulnerabilities. Maybe looking at how Grype solved this issue could help: They integrate reports/sources from the most popular distro into their database: https://github.com/anchore/grype#grypes-database sqlite> .schema vulnerability
CREATE TABLE `vulnerability` (
`pk` integer,
`id` text,
`package_name` text,
`namespace` text,
`version_constraint` text,
`version_format` text,
`cpes` text DEFAULT null,
`related_vulnerabilities` text DEFAULT null,
`fixed_in_versions` text DEFAULT null,
`fix_state` text,
`advisories` text DEFAULT null,
PRIMARY KEY (`pk`)
);
sqlite> SELECT * FROM vulnerability WHERE id='CVE-2019-1543' AND namespace LIKE '%[ubuntu](ubuntu:19.04)%';
1503117|CVE-2019-1543|openssl|ubuntu:distro:ubuntu:19.04|< 1.1.1b-1ubuntu2.2|dpkg||[{"id":"CVE-2019-1543","namespace":"nvd:cpe"}]|["1.1.1b-1ubuntu2.2"]|fixed| |
Ubuntu 22.04 with current ruby package ( |
Regarding Debian packages No regexp can solve this. Unsure if there is any easy fix. |
This is unfortunatly still an issue for us. Any recommendations on how to handle it? We are currently marking those findings as false positives. But this task is very hard to do for all of those findings. |
I am also seeing the current regexp as missing lots of potential version matching. An improved matching would be somethin along the lines of As for better correlation, I was hoping that using OSV for Debian/Ubuntu would surface more accurate results but I have yet to figure out if OSV works properly in DT for Debian purl components. |
@calderonth : I filed a bug on OSV regarding fetching Ubuntu packages by purl (google/osv.dev#2842 , with also an example for a Debian purl and some others purl examples) which was fixed that week. So i can
fetch the known CVE'S (CVE-2020-6097, CVE-2021-46671, CVE-2021-41054). Do you think using your enhanced version of your proposed regex (^([0-9]+:)?([\d.[a-z]+)(+|-)(\d$|git\w+|dfsg|\d(ubuntu|+~)).*$ , but I think this does not cover the ~0.18.04.1 portion) would solve this issue? |
Ok, I got already a script (based on python-apt) extracting all versions used by running Ubuntu distro:
In the attachment you will find the result for Ubuntu 22.04, with one version number per line for all packages. Based on that i try to find a regex matching them all. |
Nice thanks for looking into it.
I am wonder if there's prior art in such version parsing in other
projects/vulnerability scanners.
I am a bit weary of regex for this as it might be brittle given there's a
lot of leeway into how one can define package versions.
It'd be nice not to reinvent the wheel and find something that's reliable
with good unit test coverage maybe from other projects.
…On Sat, 7 Dec 2024, 13:18 Andre-85, ***@***.***> wrote:
Ok, I got already a script (based on python-apt) extracting all versions
used by running Ubuntu distro:
#!/usr/bin/python3
import apt
import sys
cache = apt.Cache()
# Iterate through each package in the cache
for package in cache:
print(package.name, file=sys.stderr)
# Get all available versions of the package
for version in package.versions:
print(version.version)
In the attachment you will find the result for Ubuntu 22.04, with one
version number per line for all packages. Based on that i try to find a
regex matching them all.
ubuntu_versions.txt
<https://github.com/user-attachments/files/18048515/ubuntu_versions.txt>
—
Reply to this email directly, view it on GitHub
<#1374 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAF3DVS3Q3JUJ7OEMZB33AT2ELYTPAVCNFSM6AAAAABRAJHXXCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMRVGEYTCMBQGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Yes, you are right, reinventing the wheel does not make sense. Right now i got different implementations in mind which solve at least the version ordering problem already:
with both you can check if your version is in a version range. Tomorrow i will start looking how this is accomplished and what regexes can be deducted from these algoriths. Further i will look for other implementations, best with already a regex for version parsing in it. |
I finally found a implementation with version ordering and regex:
I think combining the regexes for 2. and 3. and compare them with the output of algorithm in 1. and test data from ubuntu_versions.txt should lead to a usable result. I noticed that the regex used in lintian do not make any assumptions (except "ubuntu") about strings in the version number. I think this makes sense, because on a quick search in version numbers in ubuntu_versions.txt I found already 55 different strings (words_in_version_numbers.txt) My plan would be:
So overall this would be an algorithm for comparing versions like SEMVER (sematic version) but with more 'splitters' and the tilde exception What to you think? Is this doable and does it fit in the current implementation? |
After thinking a bit more about the algorithm i think i figured already out where the the problem with the openssl version 1.1.1f is. If i understood right the compareTo function of the ComponentVersion class is used (Link:
In this function each version part is first tried to convert to an integer. If the conversion was succesful an integer compare is done otherwise an string compare (I assume alphabetic ordering) is done. I my last comment i wrote that the fields should be sorted in the way like sort -n does. This is true for the current algorithm in cases where the fields are either integers OR strings, but not combined integers and strings! Simple string compare (this is what the algorithm and sort without -n (numeric sort option would do):
Thats obviously wrong! Version 1.100n.2 is not older than 1.2n.2 We should do like sort -n does:
That`s correct: Version 1.2n.2 is older than 1.100n.2 First idea which came to my mind was splitting the mixed (multiple integers and strings mixed ) fields further, so that they become a list of pure integer(s) and string(s) fields and let the algorithm run on that. What do you think? |
Hi there @Andre-85 , Thanks for looking into this. Maybe the best way to deal with this is to compare digits first and if equal compare the converted hex value of the letters? (that would work for OpenSSL) but not sure it would necessarily apply to other packages (again not sure how broad an issue this is). |
Hi @calderonth, Looking a the formal definition of semantic version 2.0.0 on https://semver.org/), you can see that there are in fact three different types of separators which split the version string into fields:
Thinking further about what the splitters are in different versioning schemes, i thought about my comment i did about spliting mixed integer+string fields further. I realized that there are not So it total this leads to following summary:
based on this i will come up with an enhanced version parsing (for CURRENT+OPENSSL and ECOSYSTEM:Ubuntu-epoch) algorithm next days. Stay tuned! |
I made progress. The implementation in ComponentVersion right now defines what characters are allowed in the version fields, while my theory about splitters is about which characters are now allowed in the fields. But for doing the Bugfix for the openssl version number we just need to apply the extra post-splitter 1->A (extra from CURRENT to CURRENT+OPENSSL) to every field definition. The current code in https://github.com/DependencyTrack/dependency-track/blob/master/src/main/java/org/dependencytrack/util/ComponentVersion.java#L92 is:
So when i am splitting all or-ed fields with 1->A, i.e. when it changes from digit to non-digit and i remove the doublicated field definitions (here \d+) this results in:
**This should fix your problem! ** I did not test it by my own, so it would be nice if you can test. But by only splitting the whole version string without looking what type of splitter was in front of the field and always prefering longer version strings only post-splitters can be implemented. I will do a new algorithm in the following days which includes SEMVER and ECOSYSTEM:Ubuntu and will post as soon i got something ready. Greetings and happy testing, |
IMO it would be best to not rely on a single regex to cover all cases. Versioning is simply too wild-west for that to work reliably. Instead we should perform ecosystem-specific version comparison if possible (#2826). Here's an implementation of the Debian version comparison, which also works for Ubuntu: https://github.com/nscuro/versatile/blob/main/versatile-core/src/main/java/io/github/nscuro/versatile/version/DebianVersion.java If a |
Looks like at least Debian provides real data which serves to identify fixed/unfixed CVE for packages: This could make pattern matching much easier and less brittle. Here's a snippet a for openssl package:
|
@nscuro : Wow, you already did a lot! I was not aware that a sophisticated java library for version comparing exists and for some reason I did not discover issue #2826.
I would be ok with all three options (but option 2 less than option 1 and option 3), which one you prefer? Or something else? |
@stmllr : You got still the problem with version comparing here. As far as I know most JSON-fied CVEs hold only a range of version (for brevitiy i think), which the CVE is present but not all versions one-by-one which are affected. In the example given by you versions later then "1.1.1n-0+deb11u4" (content of "fixed_version") do not have the CVE-2022-4450, but older ones got. Now imagine that you installed Debian bookworm on a machine which can for what-ever reason not pull automatically updates from the internet (e.g. a embedded device, a IoT device, ...). The installation you did a while ago and in this point of time the latest version of openssl in bookworm was 1.1.1k-0+deb11u1 (don't know if this version really exists). After the installation is done you pull the SBOM from the device (e.g. with apt-sbom). Today DependencyTrack/ pulls that CVE-JSON snipped. Now the decission need to be made: Is 1.1.1k-0+deb11u1 older or newer than 1.1.1n-0+deb11u4? Is CVE-2022-4450 contained or not? And this is why we need version compare in any case... |
Signed-off-by: Andre Wagner <[email protected]>
Current Behavior:
We have a cycloneDX BOM file, as generated by syft for our Docker containers. These Docker containers contain the OpenSSL library.
This is contained in the BOM file as follows:
When this BOM is uploaded, we get a number of vulnerabilities reported, including CVE-2019-1543. This CVE is stated on NVD as:
"Fixed in OpenSSL 1.1.1c (Affected 1.1.1-1.1.1b). Fixed in OpenSSL 1.1.0k (Affected 1.1.0-1.1.0j).",
Similarly we found this info in the database as follows:
However, still dependency track reports that the OpenSSL 1.1.1f-1ubuntu2.4 is vulnerable against this CVE.
Judging from the info dependency track has, it should not mark this one as a vulnerability.
Steps to Reproduce:
OR:
Expected Behavior:
Environment:
Additional Details:
After inspection of the code, I suspect that the
ComponentVersion
is the culprit here.If it parses the version
1.1.1f
it extracts[ '1', '1', '1f' ]
, while if we give it the version1.1.1f-1ubuntu2.4
it will extract['1', '1', '1', '1', 'ubuntu2', '10']
which is incorrect.I have a dirty patch for parsing the
ComponentVersion
as follows, but I did not submit it as a merge request yet since I'm not sure if it handles all cases correctly:The text was updated successfully, but these errors were encountered: