You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
(note: I have read issue #1224; this discussion is about the same problem discussed in that issue)
I became aware of this issue because I am trying to contribute to OWASP Threat Dragon and also trying to get it in shape where my work would allow its use internally. One of the immediate problems I had to contend with is that when you run npm audit on Threat Dragon, it immediately logs a critical vulnerability because of the inclusion of libxmljs2.
cyclonedx-npm-node uses libxmljs2 for some xml parsing related tasks. As such, libxmljs2 is in the dependency list for cyclonedx-npm-node, and Threat Dragon uses cyclonedx, thereby causing libxmljs2 to be included.
Because of this dependency, whenever cyclonedx-npm-node is added to any project, then any audit of such project will start causing critical security vulnerability reports when inspected with npm audit.
libxmljs2 started as a minimally supported fork of libxmljs. Both projects are no longer maintained. There exist other alternatives such as fast-xml-parser; it's not clear if any of these would meet the needs of cyclonedx-npm-node. As a security practitioner, I would strongly argue against dependencies on unmaintained packages, simply because they are unmaintained. Whether one believes the package is currently vulnerable or not, it's better to replace it when you have time, than have to scramble if a real vulnerability is found later.
Earlier today I proposed a workaround and created pull request #1286; the request was rejected. The stated reasons were:
There is no vulnerability; just a feature that can be used incorrectly (separately in issue Dependency on Library with Vulnerability #1224 the maintainer elaborates on this and also indicate that cyclonedx-node-npm does not use the feature in the insecure way, and that the maintainer has added unit tests to ensure that does not regress)
The CVE advisory on the issue is incorrect and based on untrustworthy reports by untrustworthy actors
The proposed PR changes the packaging in an unacceptable way
The proposed PR does not work
While response (3) is interesting and warrants more discussion, (4) is sufficient reason to reject the pull request. I would like to understand what doesn't work about the PR but that is a side issue. I will pass on commenting on (2) as it doesn't advance the discussion.
My point is that (1) might be correct and yet still completely misses the point of the problem you're causing for your users.
The problem for users of cyclonedx-node-npm is that npm audit reports a critical vulnerability wherever cyclonedx-node-npm is installed. The problem is not whether or not libxmljs2 actually has a vulnerability, or even whether the vulnerability can be exploited in the cyclonedx implementation. In good faith I will stipulate to the maintainer's assertions regarding exploitability.
It literally does not matter whether the vulnerability is real. This may seem like the core issue but it is really irrelevant here.
Automated vulnerability reports are trivially triggered and detected by many systems such as GitHub Dependabot, and create extra work for users who add cyclonedx to their projects, ironically to help identify dependencies on vulnerable software. It is not a good use of most people's time to try to have a technical argument with an auditor or manager why this particular critical vulnerability is not a real issue. "All our security automation says this" vs. "some guy on the internet says that" is not a winning argument, even if the guy on the internet is correct.
You would be doing many of the users of this package a huge favor to simply cause the vulnerability not to be flagged, regardless of whether it's real or not.
I'm perfectly fine if you don't like my PR. I would be extremely happy to help come up with a way to cause the vulnerability report not to be flagged, but even if you don't want my help, I would suggest that you try to find a way to prevent the report. Maybe the right thing to do is to remove libxmljs2 entirely and use a currently maintained package instead.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
Uh oh!
There was an error while loading. Please reload this page.
-
(note: I have read issue #1224; this discussion is about the same problem discussed in that issue)
I became aware of this issue because I am trying to contribute to OWASP Threat Dragon and also trying to get it in shape where my work would allow its use internally. One of the immediate problems I had to contend with is that when you run npm audit on Threat Dragon, it immediately logs a critical vulnerability because of the inclusion of libxmljs2.
cyclonedx-npm-node uses libxmljs2 for some xml parsing related tasks. As such, libxmljs2 is in the dependency list for cyclonedx-npm-node, and Threat Dragon uses cyclonedx, thereby causing libxmljs2 to be included.
Because of this dependency, whenever cyclonedx-npm-node is added to any project, then any audit of such project will start causing critical security vulnerability reports when inspected with npm audit.
libxmljs2 started as a minimally supported fork of libxmljs. Both projects are no longer maintained. There exist other alternatives such as fast-xml-parser; it's not clear if any of these would meet the needs of cyclonedx-npm-node. As a security practitioner, I would strongly argue against dependencies on unmaintained packages, simply because they are unmaintained. Whether one believes the package is currently vulnerable or not, it's better to replace it when you have time, than have to scramble if a real vulnerability is found later.
Earlier today I proposed a workaround and created pull request #1286; the request was rejected. The stated reasons were:
While response (3) is interesting and warrants more discussion, (4) is sufficient reason to reject the pull request. I would like to understand what doesn't work about the PR but that is a side issue. I will pass on commenting on (2) as it doesn't advance the discussion.
My point is that (1) might be correct and yet still completely misses the point of the problem you're causing for your users.
The problem for users of cyclonedx-node-npm is that npm audit reports a critical vulnerability wherever cyclonedx-node-npm is installed. The problem is not whether or not libxmljs2 actually has a vulnerability, or even whether the vulnerability can be exploited in the cyclonedx implementation. In good faith I will stipulate to the maintainer's assertions regarding exploitability.
It literally does not matter whether the vulnerability is real. This may seem like the core issue but it is really irrelevant here.
Automated vulnerability reports are trivially triggered and detected by many systems such as GitHub Dependabot, and create extra work for users who add cyclonedx to their projects, ironically to help identify dependencies on vulnerable software. It is not a good use of most people's time to try to have a technical argument with an auditor or manager why this particular critical vulnerability is not a real issue. "All our security automation says this" vs. "some guy on the internet says that" is not a winning argument, even if the guy on the internet is correct.
You would be doing many of the users of this package a huge favor to simply cause the vulnerability not to be flagged, regardless of whether it's real or not.
I'm perfectly fine if you don't like my PR. I would be extremely happy to help come up with a way to cause the vulnerability report not to be flagged, but even if you don't want my help, I would suggest that you try to find a way to prevent the report. Maybe the right thing to do is to remove libxmljs2 entirely and use a currently maintained package instead.
Beta Was this translation helpful? Give feedback.
All reactions