Skip to content

Commit 9e71953

Browse files
committed
Fix bug and add regression tests
Fix bug by checking HAS_PERMS_BY_NAME over the sys.all_columns view. This is safe on all supported platforms (including Synapse Analytics dedicated and shared pools - documentation is inconsistent.) Regression tests create a low-privileged login and user in on-premise environments, then use those credentials to perform a SqlBulkCopy.
1 parent f03e34f commit 9e71953

5 files changed

Lines changed: 258 additions & 25 deletions

File tree

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs

Lines changed: 34 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -477,11 +477,12 @@ private string CreateInitialQuery()
477477
}
478478
else if (!string.IsNullOrEmpty(CatalogName))
479479
{
480-
CatalogName = SqlServerEscapeHelper.EscapeStringAsLiteral(SqlServerEscapeHelper.EscapeIdentifier(CatalogName));
480+
CatalogName = SqlServerEscapeHelper.EscapeIdentifier(CatalogName);
481481
}
482482

483483
string objectName = ADP.BuildMultiPartName(parts);
484484
string escapedObjectName = SqlServerEscapeHelper.EscapeStringAsLiteral(objectName);
485+
string catalogNameStringLiteral = CatalogName is null ? null : SqlServerEscapeHelper.EscapeStringAsLiteral(CatalogName);
485486
// Specify the column names explicitly. This is to ensure that we can map to hidden
486487
// columns (e.g. columns in temporal tables.) If the target table doesn't exist,
487488
// OBJECT_ID will return NULL and @Column_Names will remain non-null. The subsequent
@@ -526,6 +527,11 @@ private string CreateInitialQuery()
526527
// we use STRING_AGG in that case and the COALESCE method otherwise.
527528
//
528529
// See: https://learn.microsoft.com/en-us/sql/t-sql/functions/serverproperty-transact-sql
530+
//
531+
// All of this is wrapped in an test against HAS_PERMS_BY_NAME. This test verifies that
532+
// the user possesses the necessary permissions to access sys.all_columns. If they do not
533+
// @Column_Names will remain NULL (and be coalesced to *) and SqlBulkCopy will degrade
534+
// gracefully, silently dropping support for hidden columns and column aliases.
529535
return $"""
530536
SELECT @@TRANCOUNT;
531537
@@ -535,6 +541,7 @@ private string CreateInitialQuery()
535541
DECLARE @Column_Name_Query_SORT NVARCHAR(MAX);
536542
DECLARE @Column_Name_Query NVARCHAR(MAX);
537543
DECLARE @Column_Names NVARCHAR(MAX) = NULL;
544+
DECLARE @Has_Permissions INT = HAS_PERMS_BY_NAME('{catalogNameStringLiteral}.[sys].[all_columns]', 'OBJECT', 'SELECT');
538545
539546
CREATE TABLE #Column_Aliases
540547
(
@@ -554,28 +561,35 @@ IF CAST(SERVERPROPERTY('EngineEdition') AS INT) = 6
554561
SET @Column_Name_Query_SORT = N'ORDER BY [column_id] ASC';
555562
END
556563
557-
IF EXISTS (SELECT TOP 1 * FROM sys.all_columns WHERE [object_id] = OBJECT_ID('sys.all_columns') AND [name] = 'graph_type')
558-
BEGIN
559-
SET @Column_Name_Query_FILTER = N'WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) NOT IN (1, 3, 4, 6, 7)';
560-
561-
EXEC sp_executesql N'
562-
INSERT INTO #Column_Aliases ([Canonical_Column_Name], [Canonical_Column_Id], [Aliased_Column_Name])
563-
SELECT [name], [column_id], ''$to_id'' FROM {CatalogName}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 8
564-
UNION ALL
565-
SELECT [name], [column_id], ''$from_id'' FROM {CatalogName}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 5
566-
UNION ALL
567-
SELECT [name], [column_id], ''$edge_id'' FROM {CatalogName}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 2 AND [name] LIKE ''$edge[_]id[_]%''
568-
UNION ALL
569-
SELECT [name], [column_id], ''$node_id'' FROM {CatalogName}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 2 AND [name] LIKE ''$node[_]id[_]%''',
570-
N'@Object_ID INT', @Object_ID = @Object_ID
571-
END
572-
ELSE
564+
IF @Has_Permissions = 1
573565
BEGIN
574-
SET @Column_Name_Query_FILTER = N'WHERE [object_id] = @Object_ID';
566+
IF EXISTS (SELECT TOP 1 * FROM {CatalogName}.[sys].[all_columns] WHERE [object_id] = OBJECT_ID('{catalogNameStringLiteral}.[sys].[all_columns]') AND [name] = 'graph_type')
567+
BEGIN
568+
SET @Column_Name_Query_FILTER = N'WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) NOT IN (1, 3, 4, 6, 7)';
569+
570+
EXEC sp_executesql N'
571+
INSERT INTO #Column_Aliases ([Canonical_Column_Name], [Canonical_Column_Id], [Aliased_Column_Name])
572+
SELECT [name], [column_id], ''$to_id'' FROM {catalogNameStringLiteral}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 8
573+
UNION ALL
574+
SELECT [name], [column_id], ''$from_id'' FROM {catalogNameStringLiteral}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 5
575+
UNION ALL
576+
SELECT [name], [column_id], ''$edge_id'' FROM {catalogNameStringLiteral}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 2 AND [name] LIKE ''$edge[_]id[_]%''
577+
UNION ALL
578+
SELECT [name], [column_id], ''$node_id'' FROM {catalogNameStringLiteral}.[sys].[all_columns] WHERE [object_id] = @Object_ID AND COALESCE([graph_type], 0) = 2 AND [name] LIKE ''$node[_]id[_]%''',
579+
N'@Object_ID INT', @Object_ID = @Object_ID
580+
END
581+
ELSE
582+
BEGIN
583+
SET @Column_Name_Query_FILTER = N'WHERE [object_id] = @Object_ID';
584+
END
585+
SET @Column_Name_Query = @Column_Name_Query_SELECT + ' FROM {catalogNameStringLiteral}.[sys].[all_columns] ' + @Column_Name_Query_FILTER + ' ' + @Column_Name_Query_SORT + ';'
586+
587+
EXEC sp_executesql @Column_Name_Query, N'@Object_ID INT, @Column_Names NVARCHAR(MAX) OUTPUT', @Object_ID = @Object_ID, @Column_Names = @Column_Names OUTPUT;
588+
589+
DELETE FROM #Column_Aliases
590+
WHERE [Aliased_Column_Name] IN (SELECT [name] FROM {CatalogName}.[sys].[all_columns] WHERE [object_id] = @Object_ID)
575591
END
576-
SET @Column_Name_Query = @Column_Name_Query_SELECT + ' FROM {CatalogName}.[sys].[all_columns] ' + @Column_Name_Query_FILTER + ' ' + @Column_Name_Query_SORT + ';'
577592
578-
EXEC sp_executesql @Column_Name_Query, N'@Object_ID INT, @Column_Names NVARCHAR(MAX) OUTPUT', @Object_ID = @Object_ID, @Column_Names = @Column_Names OUTPUT;
579593
SELECT @Column_Names = COALESCE(@Column_Names, '*');
580594
581595
SET FMTONLY ON;
@@ -586,7 +600,6 @@ UNION ALL
586600
587601
SELECT [Canonical_Column_Name], [Aliased_Column_Name]
588602
FROM #Column_Aliases
589-
WHERE [Aliased_Column_Name] NOT IN (SELECT [name] FROM {CatalogName}.[sys].[all_columns] WHERE [object_id] = @Object_ID)
590603
ORDER BY [Canonical_Column_Id] ASC
591604
592605
DROP TABLE #Column_Aliases
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
#if NETFRAMEWORK
6+
7+
namespace System.Diagnostics.CodeAnalysis;
8+
9+
#nullable enable
10+
11+
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property, Inherited = false, AllowMultiple = true)]
12+
internal sealed class MemberNotNullAttribute : Attribute
13+
{
14+
public MemberNotNullAttribute(string member) => Members = [member];
15+
16+
public MemberNotNullAttribute(params string[] members) => Members = members;
17+
18+
public string[] Members { get; }
19+
}
20+
#endif

src/Microsoft.Data.SqlClient/tests/ManualTests/Microsoft.Data.SqlClient.ManualTests.csproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
<Compile Include="DataCommon\SystemDataResourceManager.cs" />
4646
<Compile Include="DataCommon\UsernamePasswordProvider.cs" />
4747
<Compile Include="DataCommon\XEventScope.cs" />
48+
<Compile Include="Extensions\CodeAnalysis.netfx.cs" />
4849
<Compile Include="Extensions\StreamExtensions.netfx.cs" />
4950
<Compile Include="SQL\Common\AsyncDebugScope.cs" />
5051
<Compile Include="SQL\Common\ConnectionPoolWrapper.cs" />
@@ -57,6 +58,7 @@
5758
<Compile Include="SQL\Common\SystemDataInternals\FedAuthTokenHelper.cs" />
5859
<Compile Include="SQL\Common\SystemDataInternals\TdsParserHelper.cs" />
5960
<Compile Include="SQL\Common\SystemDataInternals\TdsParserStateObjectHelper.cs" />
61+
<Compile Include="SQL\SqlBulkCopyTest\UnprivilegedLogin.cs" />
6062
<Compile Include="XUnitAssemblyAttributes.cs" />
6163

6264
<!-- Content files -->

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/SqlBulkCopyTest/CopyAllFromReader.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,10 @@ public static void Test(string srcConstr, string dstConstr, string dstTable)
4040
using (DbDataReader reader = srcCmd.ExecuteReader())
4141
{
4242
IDictionary stats;
43-
long expectedIduCount = DataTestUtility.IsAzureSynapse || DataTestUtility.IsAtLeastSQL2017() ? 1 : 0;
44-
long expectedSelectCount = DataTestUtility.IsAzureSynapse ? 4 : 12;
45-
long expectedSelectRows = DataTestUtility.IsAzureSynapse ? 4 : 14;
46-
long expectedTransactions = DataTestUtility.IsAzureSynapse || DataTestUtility.IsAtLeastSQL2017() ? 1 : 0;
43+
long expectedIduCount = DataTestUtility.IsAzureSynapse || DataTestUtility.IsAtLeastSQL2017() ? 2 : 0;
44+
long expectedSelectCount = DataTestUtility.IsAzureSynapse ? 4 : 13;
45+
long expectedSelectRows = DataTestUtility.IsAzureSynapse ? 4 : 15;
46+
long expectedTransactions = DataTestUtility.IsAzureSynapse || DataTestUtility.IsAtLeastSQL2017() ? 2 : 0;
4747
using (SqlBulkCopy bulkcopy = new SqlBulkCopy(dstConn))
4848
{
4949
bulkcopy.DestinationTableName = dstTable;
Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using System;
6+
using System.Data;
7+
using System.Diagnostics.CodeAnalysis;
8+
using System.Runtime.CompilerServices;
9+
using Microsoft.Data.SqlClient.Tests.Common.Fixtures.DatabaseObjects;
10+
using Xunit;
11+
12+
namespace Microsoft.Data.SqlClient.ManualTesting.Tests.SqlBulkCopyTests;
13+
14+
#nullable enable
15+
16+
/// <summary>
17+
/// Some unprivileged users may not have permissions to query sys.all_columns, which is used to
18+
/// handle column aliases. These tests verify that bulk copy operations can handle such situations
19+
/// gracefully, falling back and ignoring column aliases.
20+
/// </summary>
21+
public sealed class UnprivilegedLogin : IDisposable
22+
{
23+
private readonly SqlConnection? _managementConnection;
24+
private readonly ServerLogin? _unprivilegedLogin;
25+
private readonly DatabaseUser? _unprivilegedMasterUser;
26+
private readonly DatabaseUser? _unprivilegedAppUser;
27+
28+
private readonly string? _unprivilegedConnectionString;
29+
30+
public static bool CanRunTests =>
31+
DataTestUtility.AreConnStringsSetup() && DataTestUtility.IsNotAzureServer()
32+
&& DataTestUtility.CanCreateLogins && DataTestUtility.CanUseSqlAuthentication;
33+
34+
public UnprivilegedLogin()
35+
{
36+
// xUnit will instantiate the class before evaluating the test condition - make sure that we don't
37+
// attempt to make use of these capabilities if the server doesn't support them.
38+
// This has the expected nullability implications, so AssertEnvironmentCreated needs to be called
39+
// before any test logic. The compiler asserts these for us.
40+
if (!CanRunTests)
41+
{
42+
return;
43+
}
44+
45+
// We require two connections: a "management" connection which has permissions to create logins,
46+
// create users and modify permissions; and an "unprivileged" connection, which is used to perform
47+
// the actual tests. The user associated with the latter connection will be denied SELECT permissions
48+
// over master.sys.all_columns.
49+
_managementConnection = new SqlConnection(DataTestUtility.TCPConnectionString);
50+
_managementConnection.Open();
51+
52+
_unprivilegedLogin = new ServerLogin(_managementConnection, nameof(UnprivilegedLogin), _managementConnection.Database);
53+
_unprivilegedAppUser = new DatabaseUser(_managementConnection, _managementConnection.Database, _unprivilegedLogin);
54+
_unprivilegedMasterUser = new DatabaseUser(_managementConnection, "master", _unprivilegedLogin);
55+
56+
using (SqlCommand permissionsModificationCommand = _managementConnection.CreateCommand())
57+
{
58+
permissionsModificationCommand.CommandText = $"DENY SELECT ON [master].[sys].[all_columns] TO {_unprivilegedMasterUser.Name}";
59+
permissionsModificationCommand.ExecuteNonQuery();
60+
61+
permissionsModificationCommand.CommandText = $"DENY SELECT ON [{_managementConnection.Database}].[sys].[all_columns] TO {_unprivilegedMasterUser.Name}";
62+
permissionsModificationCommand.ExecuteNonQuery();
63+
}
64+
65+
SqlConnectionStringBuilder tcpConnectionBuilder = new(DataTestUtility.TCPConnectionString)
66+
{
67+
IntegratedSecurity = false,
68+
UserID = _unprivilegedLogin.UnescapedName,
69+
Password = _unprivilegedLogin.Password,
70+
// Disable connection pooling - we'll be dropping this login once the tests complete, and this can only happen once all connections
71+
// using it are closed.
72+
Pooling = false
73+
};
74+
75+
_unprivilegedConnectionString = tcpConnectionBuilder.ConnectionString;
76+
}
77+
78+
/// <summary>
79+
/// This method enables nullability assertions to operate as expected. It'll always pass if CanRunTests is
80+
/// true - the constructor will have initialized the relevant fields.
81+
/// </summary>
82+
[MemberNotNull(nameof(_managementConnection),
83+
nameof(_unprivilegedLogin),
84+
nameof(_unprivilegedMasterUser),
85+
nameof(_unprivilegedAppUser),
86+
nameof(_unprivilegedConnectionString))]
87+
private void AssertEnvironmentCreated()
88+
{
89+
Assert.NotNull(_managementConnection);
90+
Assert.NotNull(_unprivilegedLogin);
91+
Assert.NotNull(_unprivilegedMasterUser);
92+
Assert.NotNull(_unprivilegedAppUser);
93+
Assert.NotNull(_unprivilegedConnectionString);
94+
}
95+
96+
/// <summary>
97+
/// Verifies that a bulk copy operation succeeds when performed by a user with only SELECT and INSERT permissions,
98+
/// without requiring access to metadata views.
99+
/// </summary>
100+
[ConditionalFact(nameof(CanRunTests))]
101+
public void BulkCopyWithoutMetadataPermission_Succeeds()
102+
{
103+
AssertEnvironmentCreated();
104+
105+
using DataTable srcDataTable = new()
106+
{
107+
Columns = { new DataColumn("Description", typeof(string)) }
108+
};
109+
using Table dstTable = new(_managementConnection, nameof(BulkCopyWithoutMetadataPermission_Succeeds), "([Description] VARCHAR(100))");
110+
using (SqlCommand permissionsConfigurationCommand = new($"GRANT SELECT, INSERT ON {dstTable.Name} TO {_unprivilegedAppUser.Name}", _managementConnection))
111+
{
112+
permissionsConfigurationCommand.ExecuteNonQuery();
113+
}
114+
115+
using SqlBulkCopy nodeCopy = new(_unprivilegedConnectionString);
116+
117+
for (int i = 0; i < 5; i++)
118+
{
119+
srcDataTable.Rows.Add($"Description {i}");
120+
}
121+
122+
nodeCopy.DestinationTableName = dstTable.Name;
123+
nodeCopy.ColumnMappings.Add("Description", "Description");
124+
nodeCopy.WriteToServer(srcDataTable);
125+
}
126+
127+
[ConditionalFact(nameof(CanRunTests))]
128+
public void BulkCopyWithoutMetadataPermission_FailsWhenUsingAliases()
129+
{
130+
AssertEnvironmentCreated();
131+
132+
using DataTable edges = new DataTable()
133+
{
134+
Columns = { new DataColumn("To_ID", typeof(string)), new DataColumn("From_ID", typeof(string)), new DataColumn("Description", typeof(string)) }
135+
};
136+
137+
// Use the management connection to create the source and the destination tables, grant the
138+
// unprivileged user permissions over them and insert some sample data into the source table.
139+
using Table srcNodeTable = new(_managementConnection, nameof(BulkCopyWithoutMetadataPermission_FailsWhenUsingAliases), "([Name] VARCHAR(100)) AS NODE");
140+
using Table dstEdgeTable = new(_managementConnection, nameof(BulkCopyWithoutMetadataPermission_FailsWhenUsingAliases), "([Description] VARCHAR(100)) AS EDGE");
141+
using (SqlCommand permissionsConfigurationCommand = _managementConnection.CreateCommand())
142+
{
143+
permissionsConfigurationCommand.CommandText = $"GRANT SELECT, INSERT ON {srcNodeTable.Name} TO {_unprivilegedAppUser.Name}";
144+
permissionsConfigurationCommand.ExecuteNonQuery();
145+
146+
permissionsConfigurationCommand.CommandText = $"GRANT SELECT, INSERT ON {dstEdgeTable.Name} TO {_unprivilegedAppUser.Name}";
147+
permissionsConfigurationCommand.ExecuteNonQuery();
148+
}
149+
150+
string sampleNodeDataCommand = @$"INSERT INTO {srcNodeTable.Name} ([Name]) SELECT LEFT([name], 100) FROM sys.sysobjects";
151+
using (SqlCommand insertSampleNodes = new(sampleNodeDataCommand, _managementConnection))
152+
{
153+
insertSampleNodes.ExecuteNonQuery();
154+
}
155+
156+
using (SqlCommand nodeQuery = new($"SELECT $node_id FROM {srcNodeTable.Name}", _managementConnection))
157+
using (SqlDataReader reader = nodeQuery.ExecuteReader())
158+
{
159+
bool firstRead = reader.Read();
160+
string toId;
161+
string fromId;
162+
163+
Assert.True(firstRead);
164+
toId = reader.GetString(0);
165+
166+
while (reader.Read())
167+
{
168+
fromId = reader.GetString(0);
169+
170+
edges.Rows.Add(toId, fromId, "Test Description");
171+
toId = fromId;
172+
}
173+
}
174+
175+
// With all source data populated, try to use the unprivileged connection to perform a bulk copy
176+
// using aliases in the column mappings. This should fail - the permissions error will be caught,
177+
// and SqlBulkCopy should simply report that the destination column doesn't exist.
178+
using SqlBulkCopy edgeCopy = new(_unprivilegedConnectionString);
179+
180+
edgeCopy.DestinationTableName = dstEdgeTable.Name;
181+
edgeCopy.ColumnMappings.Add("To_ID", "$to_id");
182+
edgeCopy.ColumnMappings.Add("From_ID", "$from_id");
183+
edgeCopy.ColumnMappings.Add("Description", "Description");
184+
185+
Action failingEdgeCopy = () => edgeCopy.WriteToServer(edges);
186+
InvalidOperationException missingColumnException = Assert.Throws<InvalidOperationException>(failingEdgeCopy);
187+
188+
Assert.Contains("'$to_id,$from_id'", missingColumnException.Message);
189+
}
190+
191+
public void Dispose()
192+
{
193+
_unprivilegedAppUser?.Dispose();
194+
_unprivilegedMasterUser?.Dispose();
195+
_unprivilegedLogin?.Dispose();
196+
_managementConnection?.Dispose();
197+
}
198+
}

0 commit comments

Comments
 (0)