Skip to content

Commit d02f4b9

Browse files
committed
Fix critical bugs: SQL injection, compilation errors, memory leaks, and security issues
- SQL Injection: Converted all SQL string concatenation to parameterized queries in SQLInteraction.cs (TransferData, TableExists, UpdateTableDesign, UpdateMetadata) - Compilation: Fixed HealthChecker.cs (removed _logger, wrong property refs) and RetryPolicy.cs (removed _logger) — both now use static Logger - Security: Removed password from console log output, removed tracked config files with production credentials from Git, updated .gitignore - Runtime: Fixed IndexOutOfRange in GetActualFields (bounds check before access), transaction corruption (fail fast on error), SPOUser disposal (added using blocks) - Resource leaks: Added IDisposable to SQLInteraction, proper rollback/cleanup - Thread safety: Lock in ConfigHelper is now actually used (double-check locking) - Minor: CAML date format (ISO 8601 T/Z), CorrelationId (full GUID), ContentTypeId (.StringValue), RetryPolicy (n+2 attempts), GetFieldValue (class constraint removed), dead code removal, OperationStatistics >= fix, Logger.LogError redundancy - Net8 fix: Pre-existing errors (SharePointOptions syntax, LinqExtensions namespace conflict, Zip overload, IList CSOM API mismatch, SPOUser conditional compilation, added System.Data.SqlClient package)
1 parent 186c2ec commit d02f4b9

20 files changed

Lines changed: 207 additions & 304 deletions

File tree

.gitignore

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -365,5 +365,5 @@ MigrationBackup/
365365
# Fody - auto-generated XML schema
366366
FodyWeavers.xsd
367367

368-
git rm --cached UserConfig.xml "UserConfig(BackUp).xml"
369-
git commit -m "Remove UserConfig files from tracking"
368+
# User-specific configuration files with sensitive credentials
369+
**/XmlConfig/UserConfig*.xml

SPOtoSQL-Net8/ConsoleApp1Net8/Application.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ private static string MaskSensitiveData(string value, int visibleChars = 3)
401401
if (value.Length <= visibleChars)
402402
return new string('*', value.Length);
403403

404-
return value[..visibleChars] + new string('*', Math.Min(6, value.Length - visibleChars));
404+
return value[..visibleChars] + new string('*', value.Length - visibleChars);
405405
}
406406

407407
/// <summary>

SPOtoSQL-Net8/ConsoleApp1Net8/Configuration/SharePointOptions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public record SharePointOptions
4141
/// Maximum retry attempts for transient failures.
4242
/// </summary>
4343
[Range(0, 10, ErrorMessage = "Max retries must be between 0 and 10")]
44-
public int MaxRetries { get; init} = 3;
44+
public int MaxRetries { get; init; } = 3;
4545

4646
/// <summary>
4747
/// Initial retry delay in milliseconds.

SPOtoSQL-Net8/ConsoleApp1Net8/ConsoleApp1Net8.csproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
<PackageReference Include="Polly" Version="8.4.2" />
4040
<PackageReference Include="Microsoft.Extensions.Diagnostics.HealthChecks" Version="8.0.8" />
4141
<PackageReference Include="Microsoft.Extensions.Http.Polly" Version="8.0.8" />
42+
<PackageReference Include="System.Data.SqlClient" Version="4.8.6" />
4243
</ItemGroup>
4344

4445
<ItemGroup>
@@ -63,7 +64,6 @@
6364
<Compile Include="..\..\SPOtoSQL-Snapshots\ConsoleApp1\Sharepoint\DataQualityBase.cs" />
6465
<Compile Include="..\..\SPOtoSQL-Snapshots\ConsoleApp1\Sharepoint\CamlQueryBuilder.cs" />
6566
<Compile Include="..\..\SPOtoSQL-Snapshots\ConsoleApp1\Sharepoint\Context.cs" />
66-
<Compile Include="..\..\SPOtoSQL-Snapshots\ConsoleApp1\Sharepoint\GetallLists.cs" />
6767
<Compile Include="..\..\SPOtoSQL-Snapshots\ConsoleApp1\Sharepoint\HealthChecker.cs" />
6868
<Compile Include="..\..\SPOtoSQL-Snapshots\ConsoleApp1\Sharepoint\InvoiceRequestDQ.cs" />
6969
<Compile Include="..\..\SPOtoSQL-Snapshots\ConsoleApp1\Sharepoint\OperationContext.cs" />

SPOtoSQL-Net8/ConsoleApp1Net8/GlobalUsings.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,7 @@
99
global using Microsoft.Extensions.Options;
1010
global using Microsoft.Extensions.Configuration;
1111
global using Microsoft.Extensions.Hosting;
12+
global using Bring.SPODataQuality;
13+
global using Bring.Sharepoint;
14+
global using Bring.Sqlserver;
15+
global using Bring.XmlConfig;

SPOtoSQL-Net8/ConsoleApp1Net8/Utilities/LinqExtensions.cs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
namespace Bring.Utilities;
1+
namespace Bring.Utilities
2+
{
23

34
/// <summary>
45
/// Demonstrates modern LINQ features introduced in .NET 6+ for SharePoint and SQL sync operations.
@@ -708,8 +709,7 @@ public static IEnumerable<TResult> DemoZip<T1, T2, T3, TResult>(
708709
IEnumerable<T3> third,
709710
Func<T1, T2, T3, TResult> resultSelector)
710711
{
711-
// Modern .NET 6+ supports 3+ sequences
712-
return first.Zip(second, third, resultSelector);
712+
return first.Zip(second, third).Select(t => resultSelector(t.First, t.Second, t.Third));
713713
}
714714

715715
#endregion
@@ -803,6 +803,8 @@ public static List<T> GetRecentChanges<T>(List<T> items, Func<T, DateTime> modif
803803
#endregion
804804
}
805805

806+
}
807+
806808
/// <summary>
807809
/// Example models for demonstrating LINQ features in SharePoint sync context.
808810
/// </summary>

SPOtoSQL-Snapshots/ConsoleApp1/ConsoleLogger/Logger.cs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,10 @@ public static void Log(int level, string message)
3838
/// </summary>
3939
public static void LogError(string message, Exception ex = null)
4040
{
41-
if (VerboseLevel >= 1)
41+
Log(1, message);
42+
if (ex != null && VerboseLevel >= 3)
4243
{
43-
Log(1, message);
44-
if (ex != null && VerboseLevel >= 3)
45-
{
46-
Log(3, $"Exception Details: {ex.GetType().Name}: {ex.Message}");
47-
}
44+
Log(3, $"Exception Details: {ex.GetType().Name}: {ex.Message}");
4845
}
4946
}
5047

SPOtoSQL-Snapshots/ConsoleApp1/SPODataQuality/RefreshSPOLists.cs

Lines changed: 26 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,6 @@ private static void RunMainWorkflow()
8181
{
8282
Logger.Log(1, "DEBUG: SPOUser created");
8383

84-
var list1 = new SPOList { SPOUser = spoUser };
85-
var list2 = new SPOList { SPOUser = spoUser };
8684
Logger.Log(3, "DEBUG: SPOList objects configured");
8785

8886
ProcessCommandLineArguments();
@@ -274,33 +272,28 @@ public static void GetAllLists()
274272
Logger.Log(1, "DEBUG: Entering GetAllLists");
275273
try
276274
{
277-
// Get SharePoint credentials
278275
var (username, password) = ConfigurationReader.GetSharePointCredentials();
279-
SPOUser spoUser = new SPOUser(username, password);
280-
// Set up a context for the SharePoint site named "seed"
281-
Context context = new Context()
276+
using (SPOUser spoUser = new SPOUser(username, password))
282277
{
283-
Site = "seed",
284-
SPOUser = spoUser
285-
};
286-
// Iterate through all lists in the SharePoint site
287-
foreach (Microsoft.SharePoint.Client.List allList in context.GetAllLists())
288-
{
289-
try
278+
Context context = new Context()
290279
{
291-
Logger.Log(1, "DEBUG: Loading list - " + allList.Title);
292-
// Load the IsSystemList property to determine if the list is a system list
293-
context.Ctx.Load<IList>(allList, new Expression<Func<Microsoft.SharePoint.Client.List, object>>[1]
294-
{
295-
l => (object) l.IsSystemList
296-
});
297-
context.Ctx.ExecuteQuery(); // Execute the query to retrieve the data
298-
Logger.Log(2, "List Name: " + allList.Title + "; is: " + allList.IsSystemList.ToString());
299-
}
300-
catch (Exception ex)
280+
Site = "seed",
281+
SPOUser = spoUser
282+
};
283+
foreach (Microsoft.SharePoint.Client.List allList in context.GetAllLists())
301284
{
302-
Console.WriteLine("ERROR: Failed to load or display list '" + allList.Title + "'.");
303-
Console.WriteLine("Exception: " + ex.Message);
285+
try
286+
{
287+
Logger.Log(1, "DEBUG: Loading list - " + allList.Title);
288+
context.Ctx.Load(allList, l => l.IsSystemList);
289+
context.Ctx.ExecuteQuery();
290+
Logger.Log(2, "List Name: " + allList.Title + "; is: " + allList.IsSystemList.ToString());
291+
}
292+
catch (Exception ex)
293+
{
294+
Console.WriteLine("ERROR: Failed to load or display list '" + allList.Title + "'.");
295+
Console.WriteLine("Exception: " + ex.Message);
296+
}
304297
}
305298
}
306299
}
@@ -522,25 +515,25 @@ private static void RefreshListsSPO(SPOList sourceList, SPOList destList)
522515
int index1 = 0;
523516
int index2 = 0;
524517

525-
// Match fields by title and store their internal names
526518
foreach (Field field1 in fields1)
527519
{
528-
Field field2;
529-
do
520+
bool found = false;
521+
while (index2 < fields2.Count)
530522
{
531-
field2 = fields2[index2];
523+
Field field2 = fields2[index2];
532524
if (field1.Title == field2.Title)
533525
{
534-
strArray[index1, 0] = field2.InternalName; // Destination field
535-
strArray[index1, 1] = field1.InternalName; // Source field
526+
strArray[index1, 0] = field2.InternalName;
527+
strArray[index1, 1] = field1.InternalName;
528+
found = true;
536529
Logger.Log(1, "DEBUG: Match found - " + field1.Title);
537530
}
538531
++index2;
532+
if (found) break;
539533
}
540-
while (field1.Title != field2.Title && index2 < fields2.Count);
541534

542535
++index1;
543-
index2 = 0; // Reset index2 for the next field
536+
index2 = 0;
544537
}
545538

546539
return strArray; // Return the field mappings

SPOtoSQL-Snapshots/ConsoleApp1/Sharepoint/CamlQueryBuilder.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,20 +59,20 @@ public static string BuildDateRangeQuery(string fieldName, DateTime from, DateTi
5959
if (to < from)
6060
throw new ArgumentException("'to' date must be greater than or equal to 'from' date.");
6161

62-
var fromValue = EscapeXmlValue(from.ToString("yyyy-MM-dd HH:mm:ss"));
63-
var toValue = EscapeXmlValue(to.ToString("yyyy-MM-dd HH:mm:ss"));
62+
var fromValue = EscapeXmlValue(from.ToUniversalTime().ToString("yyyy-MM-ddTHH:mm:ssZ"));
63+
var toValue = EscapeXmlValue(to.ToUniversalTime().ToString("yyyy-MM-ddTHH:mm:ssZ"));
6464

6565
return $@"<View>
6666
<Query>
6767
<Where>
6868
<And>
6969
<Geq>
7070
<FieldRef Name='{EscapeXmlValue(fieldName)}' />
71-
<Value Type='DateTime'>{fromValue}</Value>
71+
<Value Type='DateTime' IncludeTimeValue='TRUE'>{fromValue}</Value>
7272
</Geq>
7373
<Leq>
7474
<FieldRef Name='{EscapeXmlValue(fieldName)}' />
75-
<Value Type='DateTime'>{toValue}</Value>
75+
<Value Type='DateTime' IncludeTimeValue='TRUE'>{toValue}</Value>
7676
</Leq>
7777
</And>
7878
</Where>

SPOtoSQL-Snapshots/ConsoleApp1/Sharepoint/DataQualityBase.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,18 +134,18 @@ protected void ProcessListItemsInBatches(SPOList list, Action<ListItem> processo
134134
/// <param name="item">The list item.</param>
135135
/// <param name="fieldName">The internal field name.</param>
136136
/// <returns>The field value, or default if null or not found.</returns>
137-
protected T GetFieldValue<T>(ListItem item, string fieldName) where T : class
137+
protected T GetFieldValue<T>(ListItem item, string fieldName)
138138
{
139139
try
140140
{
141-
if (item == null) return null;
141+
if (item == null) return default;
142142
if (item[fieldName] is T value) return value;
143-
return null;
143+
return default;
144144
}
145145
catch (Exception ex)
146146
{
147147
Logger.LogDebug($"Failed to retrieve field '{fieldName}' from item: {ex.Message}");
148-
return null;
148+
return default;
149149
}
150150
}
151151

0 commit comments

Comments
 (0)