-
Notifications
You must be signed in to change notification settings - Fork 501
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
feat(checker): Detect OpenSSL 0.9.x, as found in msys/cygwin #4735
base: main
Are you sure you want to change the base?
Conversation
1732eac
to
4780eee
Compare
@terriko I've updated the tests; would you mind re-approving this workflow? |
Pinging @ffontaine -- I know you've been working on reducing false positives in this particular pattern, is this update going to cause you problems? |
4780eee
to
b505292
Compare
Thanks a lot @terriko for pinging me. For example libssh uses this piece of code to log the version of openssl that was used to compile the library:
This is an interesting information but libssh.so is a shared library, not a static one. It doesn't embed any code from openssl and so is not affected by any CVEs related to openssl. Moreover, it doesn't mean that the final openssl library that will be embedded in the system has the same version (e.g. libssh could have been compiled with openssl 3.3.2 and the final openssl version installed on the system could be openssl 3.3.4). This would be odd but could be seen on embedded systems. You can easily reproduce this false positive by compiling this simple main program:
If this PR is merged as it is, cve-bin-tool will (wrongly) detect openssl inside this binary. |
Thanks @ffontaine . The approach taken by cve-bin-tool relies on looking for version information in strings embedded in binaries. If your application includes a version string of another component, that would result in a false positive. That's not specific to OpenSSL; that's a consequence of the approach taken by this tool. You could argue that including What I found was that the tool currently does not detect OpenSSL as built by old versions of MSYS or GnuWin32. Those versions merely contain this version string: I think a false negative on an old version of OpenSSL would be an important limitation (that GnuWin32 build is unfortunately still used). Is there another approach you would suggest which would avoid this false negative? |
Hi @qmfrederik, using latest main, I added
I then run I double checked by running cve-bin-tool on So I'm not able to reproduce your issue, can you provide additional details (e.g. other binaries)? |
Ah - good catch, thanks. Let me see if I can find another package on Windows which is representative. |
@ffontaine Here's another example: https://ftp-stud.hs-esslingen.de/pub/Mirrors/sources.redhat.com/cygwin/x86_64/release/openssl10/libssl1.0/libssl1.0-1.0.2t-1.tar.xz The version string is embedded like this:
I have another example which reads like this:
And then there's the interesting example of nodejs, which (usually) contains a static copy of OpenSSL: https://github.com/nodejs/node/blob/main/doc/contributing/maintaining/maintaining-openssl.md#L3 . |
1.0.2t is correctly detected in
This is not surprising as the current multiline pattern will be triggered on Concerning nodejs, do you have a concrete example of a nodejs package statically linked with openssl?
|
When I use main, OpenSSL is not detected in
includes an underscore (via For node, yes, I'm using libnode72 which is part of Ubuntu 24.04 (http://security.ubuntu.com/ubuntu/pool/universe/n/nodejs/libnode72_12.22.9~dfsg-1ubuntu3.6_amd64.deb). That does not dynamically link with OpenSSL:
and the OpenSSL version string and the other keywords the regex is looking for appear to be pretty far from each other. |
Yes, adding Concerning nodejs, thanks for your link, this is an interesting topic. When I grep for OpenSSL, it indeeds contains Basically, I see two approaches:
I don't have any strong opinion, I'll let @terriko read all our messages. I'm quite confident that she will have helpful advice. |
As a side-effect, this also finds OpenSSL in a bunch of other libraries.