Hangfire icon indicating copy to clipboard operation
Hangfire copied to clipboard

Update to newer Newtonsoft or switch to System.Text.json

Open allenkepler-finastra opened this issue 2 years ago • 10 comments

The version of Newtonsoft referenced has known vulnerabilities. Anyone referencing this has to also reference a newer version of Newtonsoft to clear security scans. My solution is Newtonsoft free and I'd rather not add packages for my dev teams to accidentally use. If possible I think it would be better to switch to System.Text.Json or extract serialization in a similar fashion to how database was done.

Thanks

Vulnerability info: https://github.com/advisories/GHSA-5crp-9r3c-p9vr

allenkepler-finastra avatar May 04 '23 13:05 allenkepler-finastra

I'm afraid that issue belongs more to NuGet than to Hangfire and is present in other packages as well, for example, see the following question on Stack Overflow:

https://stackoverflow.com/questions/56727314/nuget-package-manager-does-not-install-package-with-highest-depencency-version

There was the DependencyVersion switch in the early days of NuGet, but I don't know how to use it with PackageReference tags. The problem is also described here – https://weblog.west-wind.com/posts/2014/Jun/19/Nuget-Dependencies-and-latest-Versions and there's an issue on GitHub in the NuGet repository that describes this issue in great detail, but unfortunately, I can't find it.

The thing is, Hangfire is an abstraction itself (since it's a framework). It does specify the minimum supported version for its dependencies, and actual versions can be specified (Hangfire will work with any of them) in the target application by specifying those dependent packages explicitly:

<ItemGroup>
    <PackageReference Include="Newtonsoft.Json" Version="*" />
</ItemGroup>

Unfortunately, if we try to implement this NuGet-based feature in the project itself, we'll be forced to specify only the latest versions almost all of the time, forcing developers into dependency hell problem, where one package requires one version, and another package requires another version of Hangfire.Core or Newtonsoft.Json.

UPD. This is a general issue with NuGet, and is tracked in https://github.com/nuget/home/issues/5553.

odinserj avatar May 05 '23 10:05 odinserj

@allenkepler-finastra Mybe instruct your dev-team to always make sure they run on newest version of all packages their project references.

burningice2866 avatar May 05 '23 10:05 burningice2866

I did find that this has been talked about a number of times here. I didn't see it at first and can't find the close issue button. Hopefully you'll consider updated to the minimal "safe" version in 2.0 or moving to System.Text.Json. As far as dependency hell goes, I've been their and done that. I've never had to do much more for newtonsoft than add binding redirects in my older net framework projects.

Thanks!

allenkepler-finastra avatar May 05 '23 12:05 allenkepler-finastra

It can be very challenging to migrate to another serialisation framework, even if we talk about JSON – there can be different ways of expressing the same things (like types using the $type directive), and I'm afraid there will be breaking changes for some projects. So this step should be made with advanced care, and perhaps moved at least to Hangfire 2.0.

odinserj avatar May 08 '23 09:05 odinserj

I've tried to migrate my project from Newtonsoft to System.Text.json. I quickly abandoned that mission ;).

sabiland avatar May 08 '23 12:05 sabiland

Yes, and there also can be some advanced configuration of the serialisation process that will be very difficult to match with System.Text.Json. Also if we even migrated to the other package, we will be required to set some early version to prevent breaking changes as well, yielding the same problems.

odinserj avatar May 09 '23 08:05 odinserj

At the very least I would like to be on Newtonsoft 13.

jttommy avatar Sep 28 '23 18:09 jttommy

@jttommy, you can use any Newtonsoft.Json version, starting from 5.0.1 by explicitly referencing this package from your project, please see https://github.com/HangfireIO/Hangfire/issues/2202#issuecomment-1536065715.

odinserj avatar Sep 29 '23 05:09 odinserj

@jttommy, you can use any Newtonsoft.Json version, starting from 5.0.1 by explicitly referencing this package from your project, please see #2202 (comment).

@odinserj I hear that, but it's quite frustrating to need to add a reference to an assembly that we ourselves don't need to use just to patch over a security vulnerability introduced by the library. At this point, all users of Newtonsoft JSON from NetStandard2.0 or higher should be using version 13.0.1 or higher, and allowing a minimum version that is well-known to be vulnerable feels a little odd, possibly irresponsible.

Just my $0.02 as someone who is now having to go in and add a ton of unnecessary references to Newtonsoft.Json in a bunch of projects that have nothing to do with Json.

JamesTerwilliger avatar Oct 20 '23 21:10 JamesTerwilliger

It can be very challenging to migrate to another serialisation framework, even if we talk about JSON – there can be different ways of expressing the same things (like types using the $type directive), and I'm afraid there will be breaking changes for some projects. So this step should be made with advanced care, and perhaps moved at least to Hangfire 2.0.

@odinserj instead of making a breaking change it would be way better to provide an extensibility point to allow different serializers (and therefore also custom ones provided by the user). For some projects it might be better to use Newtonsoft.Json (especially existing ones when they use custom json converters), for others System.Text.Json would be better, maybe some want to use XML. Looking at SerializationHelper it has a very simple public surface which essentially just consists out of string Serialize(object) and object Deserialize(string) (and some overloads here and there). From my understanding Hangfire really doesn't care about the serialized format that is written to storage.

Just as a thought, IGlobalConfiguration could expose something like UseSerializer(IJobSerializer) which could then be provided by the user. Serializers could then be moved to dedicated packages like Hangfire.Serialization.SystemTextJson and Hangfire.Serialization.JsonNet with some extension methods for IGlobalConfiguration. The Hangfire metapackage would reference Hangfire.Serialization.JsonNet and configure the Newtonsoft serializer as a default. This would provide great backwards compatibility. This would only break stuff for people not using the metapackage. However, migration is fairly simple and even easier than a schema migration.

klemmchr avatar Oct 24 '23 20:10 klemmchr