NHibernate 5.1.3 relies on version of System.ServiceModel.Primitives with a known vulnerabilities

[rheone]

NHibernate 5.1.3 for both .NETCoreApp 2.0 and .NETStandard 2.0 has a dependency on System.ServiceModel.Primitives >= 4.4.0

System.ServiceModel.Primitives 4.4.0 has known vulnerabilities. The dependency on System.ServiceModel.Primitives should have its version bumped to a non vulnerable release

The Microsoft advisories are available at:

• CVE-2018-0764
• CVE-2018-0786

Microsoft .NET Flaws Let Remote Users Deny Service and Bypass Certificate Security Restrictions

[fredericDelaporte]

NHibernate does not open itself any WCF communication channel and as such is not concerned by these vulnerabilities.

When NHibernate is used in WCF application, this is the responsibility of the application to use a non-vulnerable WCF dependency, which NHibernate will not prevent.

Anyway, the bit of code taking this dependency, the WcfOperationContext, may be moved out of NHibernate with #1820.

[rheone]

I understand what your are saying, and I get that NHibernate doesn't deal with WFC Communication (and neither does my application that relies on NHibernate), but is it really the stance of NHibernate to introduce code with known vulnerabilities into peoples projects? If so, is it documented anywhere of the known vulnerable libraries that are introduced to a project by including NHibernate?

[oskarb]

rheone,
Perhaps there is something I'm not aware of, but I fail to follow your reasoning. NHibernate merely states that it needs at least version 4.4.0 to be able to perform its function. If you add NHibernate to a project, surely NuGet would pull the latest available version of System.ServiceModel.Primitives? If you already have 4.4.0, then it's a matter of keeping up to date on NuGet.

A vulnerable version of the package you mention is not delivered as part of NHibernate but directly from NuGet, so it's not really NHibernate's responsibilty to enforce or ensure that this package is updated, is it? If it is, should NHibernate also refuse to run unless you have installed the latest monthly security fix from Windows Update? What I'm saying is, why is this particular package the responsibility of NHibernate and not the other dependencies?

[ahsteele]

@oskarb I don't want to put words in @rheone mouth, but it seems to me that there is responsibility when a package is referenced to keeping up with that package's updates. You are correct that NHibernate references System.ServiceModel.Primitives 4.4.0 or better. Since 4.4.0 has known vulnerabilities it seems that NHibernate should version step at least to a version without known vulnerabilities.

As it stands adding NHibernate to a project adds a known vulnerable dll to a code base when a project is built (irregardless if NuGet is the reason that is pulled into the project). This isn't even happening transitively as NHibernate has an explicit reference to a vulnerable version.

All of that said there have been six releases since 4.4.0. Would a better reported issue be a request to update to 4.5.3 without explaining why?

[oskarb]

As it stands adding NHibernate to a project adds a known vulnerable dll to a code base when a project is built...

But does it really? Wouldn't NuGet pull the latest available version (since NHibernate does not prevent this)?

I don't really care all that much, but I still don't understand why it's considered NHibernate's responsibility to enforce an update of this one particular dependency.

All of that said there have been six releases since 4.4.0. Would a better reported issue be a request to update to 4.5.3 without explaining why?

Does that version contain anything that NHibernate would benefit from depending on? If you want that version for some reason, just update your reference. The NHibernate reference doesn't prevent it.

Needlessly depending on the very latest version would (in general terms) be troublesome if the end developer needs an older version due to some other dependency or other reason. In general terms, we should depend on the oldest feasible version as long as a newer version doesn't contain API-changes that adds a useful benefit for NHibernate. If a later versions contains internal improvements, just run the latest version - NHibernate doesn't care.

[fredericDelaporte]

I fail to understand why that reference having some vulnerabilities that cannot be exploited on any non WCF application is a trouble.
A WCF application will anyway have a direct reference and should care itself to have it updated.

A non-WCF application simply does not have to bother: the vulnerable referenced assembly will not even get executed, since none of its features will be used.

All of that said there have been six releases since 4.4.0. Would a better reported issue be a request to update to 4.5.3 without explaining why?

References are not updated without a reason, so that would not be done.

By the way, these vulnerabilities are also fixed in 4.4.3, so there is no need to bump up to 4.5.x.

I am quite reluctant to start upgrading references for "protecting" either of nothing (non WCF-application) or protecting without being NHibernate responsibility (WCF-application), because that is still some work to do.

If having this reference pulled-in as 4.4.0 is a concern for you, you can still force it to a more recent version of your choice.

[rheone]

If you add NHibernate to a project, surely NuGet would pull the latest available version of System.ServiceModel.Primitives?

@oskarb, unless I have nuget improperly configured, I believe by default a nuget package restore functionality simply pulls the lowest compatible version of a dependency. In this case System.ServiceModel.Primitives 4.4.0 as NHibernate is the only dependency requesting it and 4.4.0 satisfies '>= 4.4.0'

...should NHibernate also refuse to run unless you have installed the latest monthly security fix from Windows Update?

Obviously not, but if NHibernate explicitly called out and included (via a direct package dependency) a known vulnerable version of windows, yeah, it would be nice to know that I'm including vulnerable code in my project on behalf of NHibernate.

What I'm saying is, why is this package the responsibility of NHibernate and not the other dependencies?

It may not be the responsibility of NHibernate, but NHibernate will by default include known vulnerable code in any project that consumes it because it has a direct dependency on said vulnerable library. As called out in the NHibernate.csproj lines 55 and 61. A rising tide lifts all boats. It is hard enough for folks to manage their own dependencies, but now all responsible consumers of NHibernate should need to manage NHibernate's dependencies as well. Even if the minimum dependency versions in NHibernate is not updated for known vulnerabilities a clear and called out documentation of known issues for direct dependencies would be handy to reference.

If having this reference pulled-in as 4.4.0 is a concern for you, you can still force it to a more recent version of your choice.

@fredericDelaporte, this is exactly what I am doing at the moment.

I was only made aware of this issue though a security scan via checkmarx. System.ServiceModel.Primitives 4.4.0 only appears in my project due to NHibernate, and it took both time and effort to track down what was causing problematic dependency. I couldn't in good conscience call it a false positive until I tracked down what was using it and how. I'm just trying to be a good opensource citizen save other developers the same headache.

[oskarb]

Perhaps I was mistaken when assuming NuGet would pull the latest version. Perhaps it pulls the lowest compatible version for the same reason I wrote about above - maximum compatibility. Then it makes a bit more sense to me to increase the version in the dependency to the lowest fixed version. I'm not opposed to it at least. :)

Won't help for all existing projects though. I suspect that if the developer decides to update NHibernate from NuGet they are fairly likely to update other NuGet packages too.

It's not really up to me to decide, but I suspect it would help if someone submitted a pull request, targetting the lowest fixed version.

[rheone]

Unfortunately, NuGet only gathers the most compatible version of dependencies if versions are not specified. Transitive dependencies aren’t even explicitly called out, so a developer that uses a library that calls a "bad" dependency they may not even be aware what's happening under the covers. As far as I'm aware a warning isn’t even flagged.

I’ll evaluate creating a pull request today.

[fredericDelaporte]

See this comment on #1820: maybe the right change is simply to remove completely this dependency under .Net Standard/.Net Core.

[rheone]

@fredericDelaporte, agreed, shaking out unused dependencies, or separating them to a focused sub library, may be the best solution when possible.

[fredericDelaporte]

NHibernate 5.2 (.Net Standard and .Net Core binaries) will no more have this dependency.