Add direct reference to System.Text.Encodings.Web version 4.7.2 due to CVE-2021-26701

[jrebagliatti]

Fixes #3735

Changes

Added direct reference to package System.Text.Encodings.Web version 4.7.2 to deal with CVE-2021-26701. This overrides dependency path to a vulnerable version: System.Text.Json/4.7.2 -> SystemText.Encodings.Web/4.7.1.

See my comment in #3789.

Merge requirement checklist

• CONTRIBUTING guidelines followed (nullable enabled, static analysis, etc.)
• Unit tests added/updated
• Appropriate CHANGELOG.md files updated for non-trivial changes
• Changes in public API reviewed (if applicable)

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: jrebagliatti / name: Javier Rebagliatti (ff08a32, 0d0c3ee)

[codecov]

Codecov Report

Merging #4390 (4558595) into main (7958763) will increase coverage by 0.00%.
The diff coverage is n/a.

❗ Current head 4558595 differs from pull request most recent head f521bd5. Consider uploading reports for the commit f521bd5 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4390   +/-   ##
=======================================
  Coverage   84.73%   84.74%           
=======================================
  Files         300      300           
  Lines       12010    12010           
=======================================
+ Hits        10177    10178    +1     
+ Misses       1833     1832    -1     

see 7 files with indirect coverage changes

[reyang]

I haven't looked into the versioning story. Do we know if 5.*.* is backward compatible with 4.7.* and if folks can just bump the version?

If the answer is no, we probably need different versions depending on the target runtime version:

[reyang]

I got some advice from the owner of System.Text.Json and System.Text.Encoding.Web:

1. The proposed version bump of System.Text.Json might give trouble to users who rely on certain features that were affected by these breaking changes.
2. There is much higher confidence that System.Text.Encoding.Web version bump is very backward compatible (so we don't expect any surprise due to breaking changes).

Given the above information, I suggest that we don't make the System.Text.Json version bump, instead, we add explicit dependency to System.Text.Encoding.Web and enforce the versions which have fixed the security vulnerabilities.

[jrebagliatti]

@reyang based on your comments, I've reverted the version bump of System.Text.Json and added a direct reference to System.Text.Encodings.Web version 4.7.2 to the following projects:

• Exporter.Console
• Exporter.Jaeger
• Exporter.Zipkin

Instrumentation.Http.Tests is also referencing System.Text.Json, but apparently it's using version 6.0.5 which references version >6.0.0 of System.Text.Encodings.Web which is not affected by the vulnerability.

I see, however, that there's at least one other library referencing a vulnerable version of System.Text.Encodings.Web:

• OpenTelemetry.Instrumentation.AspNetCore -> Microsoft.AspNetCore.Http.Abstractions/2.1.1 -> System.Text.Encodings.Web/4.5.0

In this case I think the solution should be to add a direct reference to System.Text.Encodings.Web/4.5.1 (patched version) as there's no other Microsoft.AspNetCore.Http.Abstractions/2.1.x minor release that fix the issue (actually, latest version, 2.2.0, doesn't fix it either).

If you want me to do the change, please advise how should I proceed as I'm not familiarized with the project's dependency management.

[reyang]

I see, however, that there's at least one other library referencing a vulnerable version of System.Text.Encodings.Web:

• OpenTelemetry.Instrumentation.AspNetCore -> Microsoft.AspNetCore.Http.Abstractions/2.1.1 -> System.Text.Encodings.Web/4.5.0

In this case I think the solution should be to add a direct reference to System.Text.Encodings.Web/4.5.1 (patched version) as there's no other Microsoft.AspNetCore.Http.Abstractions/2.1.x minor release that fix the issue (actually, latest version, 2.2.0, doesn't fix it either).

If you want me to do the change, please advise how should I proceed as I'm not familiarized with the project's dependency management.

@jrebagliatti I suggest that we address this in a separate PR.
@vishweshbankwar Would you look into the OpenTelemetry.Instrumentation.AspNetCore part?

[reyang]

LGTM. @jrebagliatti would you update the PR title/description to reflect the intention/change? (e.g. System.Text.Encodings.Web)

[Kielek]

@jrebagliatti, could you please update PR title to reflect current content?

[Kielek]

AutoInstrumentation - as we should avoid bumping libraries, the CVE is good explanation to do it.