[cheenamalhotra]

Fixes #4062

[Copilot]

Pull request overview

Adds an explicit System.Data.Common dependency to the SqlClient build and packaging metadata so the net462 target has a direct/declared reference instead of relying on implicit or transitive resolution.

Changes:

• Add System.Data.Common as a PackageReference for the net462 build of Microsoft.Data.SqlClient.
• Add System.Data.Common to the netfx reference-assembly project’s package references.
• Pin System.Data.Common centrally and declare it as a net462 dependency in the .nuspec.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
tools/specs/Microsoft.Data.SqlClient.nuspec	Declares System.Data.Common (4.3.0) as a net462 NuGet dependency.
src/Microsoft.Data.SqlClient/src/Microsoft.Data.SqlClient.csproj	Adds System.Data.Common package reference for the net462 target.
src/Microsoft.Data.SqlClient/netfx/ref/Microsoft.Data.SqlClient.csproj	Adds System.Data.Common package reference for the net462 reference assembly build.
Directory.Packages.props	Centrally pins System.Data.Common to 4.3.0.

---

You can also share your feedback on Copilot code review. Take the survey.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.44%. Comparing base (ce38e64) to head (e856b24).
⚠️ Report is 5 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (ce38e64) and HEAD (e856b24). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (ce38e64)	HEAD (e856b24)
CI-SqlClient	1	0

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4063      +/-   ##
==========================================
- Coverage   75.09%   68.44%   -6.66%     
==========================================
  Files         280      275       -5     
  Lines       43830    66930   +23100     
==========================================
+ Hits        32914    45807   +12893     
- Misses      10916    21123   +10207     

Flag	Coverage Δ
CI-SqlClient	?
PR-SqlClient-Project	68.44% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[mdaigle]

@copilot When were these references removed? Can you point to a specific PR?

[Copilot]

@mdaigle I've opened a new pull request, #4066, to work on those changes. Once the pull request is ready, I'll request review from you.

[mdaigle]

@copilot backport this change to the release/7.0 branch

[snakefoot]

Think this the correct fix is to use <Reference Include="System.Data.Common" /> for .NET Framework.

Notice <PackageReference Include="System.Data.Common" /> makes VeraCode report security vulnerabilities:

• CVE-2020-1147

[paulmedynski]

We use the System.Data.Common NuGet package instead of the in-box .NET Framework assembly because we need types that aren't available in .NET Framework 4.6.2. See #2967 for the original change.

@snakefoot - I'm not sure how the CVE you noted relates to this. Can you provide details from the VeraCode report that clearly explain?

[snakefoot]

NET Framework already have everything. The Microsoft.Data.SqlClient just need to use the native NET Framework-reference (Notice Reference instead PackageReference):

<Reference Include="System.Data.Common" />

As you can already see from initial issue #4062 (Nuget-package also supports "hidden" / "implicit" NET Framework references) - Reference Include="System.Transactions"

VeraCode reports that the obsolete/unmaintained System.Data.Common-nuget-package has issues: https://sca.analysiscenter.veracode.com/vulnerability-database/libraries/system-data-common/csharp/nuget/lid-1528727 CVE-2020-1147 Remote Code Execution (RCE)

• XML Deserialization Flaw: The issue occurs when .NET types like System.Data.DataSet or System.Data.DataTable read XML-serialized data without properly validating the source markup of the XML input. See also: https://vulners.com/veracode/VERACODE:25896

[paulmedynski]

@snakefoot - The SqlClient package targets .NET Framework 4.6.2 and .NET 10.0. .NET Framework 4.6.2 does not contain the following types that we require:

• System.Data.Common.DbColumn
• System.Data.Common.DbDataReaderExtensions
• System.Data.Common.IDbColumnSchemaGenerator

Those first appeared in .NET Framework 4.7.1 - see this blog.

In order to compile for .NET Framework 4.6.2, we must include the System.Data.Common 4.3.0 NuGet package that provides those 3 types. That package is not marked as deprecated or obsolete, v4.3.0 is the latest version available, and NuGet has no documented vulnerabilities listed for it.

The Veracode link you provided claims there are vulnerabilities, but doesn't disclose them. The CVE-2020-1147 doesn't list the NuGet package as far as I could see.

SqlClient targeting .NET Framework 4.6.2 requires the NuGet package and its System.Data.Common assembly supplying these types, even when building on a machine with a newer .NET Framework installed, or for apps targeting .NET Framework 4.7.1 or newer.

Are you suggesting that we attempt to modify the SqlClient nuspec file to specify something like this?

<dependencies>
  <group targetFramework="net462">
    <dependency id="System.Data.Common" version="4.3.0" />
    <!-- Other applicable dependencies -->
  </group>

  <group targetFramework="net471">
    <!-- Other applicable dependencies, but no System.Data.Common package -->
  </group>
</dependencies>

Please open a new issue with complete findings and a suggested fix (ideally with reproducible proof-of-concept) so we can continue this discussion. We will be happy to asses what you come up with, especially if it means we can avoid unnecessary warnings in downstream consumers.

[snakefoot]

Have created #4939 - Guess the solution is to stay with SqlClient v7.0.0 until you release SqlClient that supports NET 4.7.1

Notice using SqlClient v7.0.0 (without the obsolete/unmaintained System.Data.Common nuget-package) works fine with NET Framework 4.8 (Just requires one to explicit add own reference as also mentioned here #4062)