Skip to content

Commit

Permalink
[ASM] iast: Fix Microsoft.Data.Sqlite database tainting (#6295)
Browse files Browse the repository at this point in the history
## Summary of changes

- The instrumentation introduced for tainting values coming from
database was incorrect for `Microsoft.Data.Sqlite`.
A typo was introduced in the name
`Microsoft.Data.Sqlite.SqliteDataReader`, thus making the
instrumentation fail.

- Instrumentations now use the latest dotnet version supported (here
dotnet 9)

## Test coverage

Tests on database tainting such as Stored XSS or Stored SQLI has been
updated to also create an SQLite connection using
`Microsoft.Data.Sqlite` while also keeping the `System.Data.SQLite`
connection.

## Other details
Original implementation PR: #5804
  • Loading branch information
e-n-0 authored Nov 18, 2024
1 parent 9e391e6 commit 707de2c
Show file tree
Hide file tree
Showing 14 changed files with 226 additions and 162 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ namespace Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet
[EditorBrowsable(EditorBrowsableState.Never)]
public class ReaderGetStringIntegration
{
private static bool errorLogged = false;
private static bool _errorLogged;

/// <summary>
/// OnMethodBegin callback
Expand All @@ -46,7 +46,7 @@ internal static CallTargetState OnMethodBegin<TTarget>(TTarget instance, int ind
/// <param name="exception">Exception instance in case the original code threw an exception.</param>
/// <param name="state">Calltarget state value</param>
/// <returns>A response value, in an async scenario will be T of Task of T</returns>
internal static CallTargetReturn<TReturn> OnMethodEnd<TTarget, TReturn>(TTarget instance, TReturn returnValue, Exception exception, in CallTargetState state)
internal static CallTargetReturn<TReturn> OnMethodEnd<TTarget, TReturn>(TTarget instance, TReturn returnValue, Exception? exception, in CallTargetState state)
{
try
{
Expand All @@ -59,18 +59,18 @@ internal static CallTargetReturn<TReturn> OnMethodEnd<TTarget, TReturn>(TTarget
}
else
{
column = state.State?.ToString() ?? string.Empty;
column = string.Empty;
}

IastModule.AddDbValue(instance!, column, value);
}
}
catch (Exception e)
{
if (!errorLogged)
if (!_errorLogged)
{
Log.Error(e, "Error adding db value to IAST module");
errorLogged = true;
_errorLogged = true;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>

using Datadog.Trace.ClrProfiler.AutoInstrumentation;
using Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet;
using Datadog.Trace.Configuration;
using static Datadog.Trace.ClrProfiler.AutoInstrumentation.AdoNet.AdoNetClientInstrumentMethodsAttribute;
Expand All @@ -12,7 +13,7 @@
AssemblyName = "Microsoft.Data.Sqlite",
TypeName = "Microsoft.Data.Sqlite.SqliteCommand",
MinimumVersion = "2.0.0",
MaximumVersion = "8.*.*",
MaximumVersion = SupportedVersions.LatestDotNet,
IntegrationName = nameof(IntegrationId.Sqlite),
DataReaderType = "Microsoft.Data.Sqlite.SqliteDataReader",
DataReaderTaskType = "System.Threading.Tasks.Task`1[Microsoft.Data.Sqlite.SqliteDataReader]",
Expand Down Expand Up @@ -40,8 +41,8 @@
MinimumVersion = "1.0.0",
MaximumVersion = "2.*.*",
IntegrationName = nameof(IntegrationId.Sqlite),
DataReaderType = "System.Data.SQLite.SqliteDataReader",
DataReaderTaskType = "System.Threading.Tasks.Task`1[System.Data.SQLite.SqliteDataReader]",
DataReaderType = "System.Data.SQLite.SQLiteDataReader",
DataReaderTaskType = "System.Threading.Tasks.Task`1[System.Data.SQLite.SQLiteDataReader]",
TargetMethodAttributes = new[]
{
// int System.Data.SQLite.SQLiteCommand.ExecuteNonQuery()
Expand All @@ -62,12 +63,12 @@

[assembly: AdoNetClientInstrumentMethods(
AssemblyName = "Microsoft.Data.Sqlite",
TypeName = "Microsoft.Data.Sqlite.SQLiteDataReader",
TypeName = "Microsoft.Data.Sqlite.SqliteDataReader",
MinimumVersion = "2.0.0",
MaximumVersion = "8.*.*",
MaximumVersion = SupportedVersions.LatestDotNet,
IntegrationName = nameof(IntegrationId.SqlClient),
DataReaderType = "System.Data.SQLite.SQLiteDataReader",
DataReaderTaskType = "System.Threading.Tasks.Task`1[System.Data.SQLite.SQLiteDataReader]",
DataReaderType = "Microsoft.Data.Sqlite.SqliteDataReader",
DataReaderTaskType = "System.Threading.Tasks.Task`1[Microsoft.Data.Sqlite.SqliteDataReader]",
TargetMethodAttributes = new[]
{
// string System.Data.Common.DbDataReader.GetString()
Expand Down

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -220,14 +220,17 @@ await VerifyHelper.VerifySpans(spansFiltered, settings)
.DisableRequireUniquePrefix();
}

[SkippableFact]
[SkippableTheory]
[Trait("Category", "ArmUnsupported")]
[Trait("RunOnWindows", "True")]
public async Task TestIastStoredXssRequest()
[InlineData("System.Data.SQLite")]
[InlineData("Microsoft.Data.Sqlite")]
public async Task TestIastStoredXssRequest(string database)
{
var filename = "Iast.StoredXss.AspNetCore5." + (IastEnabled ? "IastEnabled" : "IastDisabled");
var useMicrosoftDataDb = database == "Microsoft.Data.Sqlite";
if (RedactionEnabled is true) { filename += ".RedactionEnabled"; }
var url = "/Iast/StoredXss?param=<b>RawValue</b>";
var url = $"/Iast/StoredXss?param=<b>RawValue</b>&useMicrosoftDataDb={useMicrosoftDataDb}";
IncludeAllHttpSpans = true;
await TryStartApp();
var agent = Fixture.Agent;
Expand All @@ -238,18 +241,23 @@ public async Task TestIastStoredXssRequest()
settings.AddIastScrubbing();
settings.AddRegexScrubber(aspNetCorePathScrubber);
settings.AddRegexScrubber(hashScrubber);
settings.AddRegexScrubber((new Regex(@"&useMicrosoftDataDb=(True|False)"), "&useMicrosoftDataDb=..."));

await VerifyHelper.VerifySpans(spansFiltered, settings)
.UseFileName(filename)
.DisableRequireUniquePrefix();
}

[SkippableFact]
[SkippableTheory]
[Trait("Category", "ArmUnsupported")]
[Trait("RunOnWindows", "True")]
public async Task TestIastStoredXssEscapedRequest()
[InlineData("System.Data.SQLite")]
[InlineData("Microsoft.Data.Sqlite")]
public async Task TestIastStoredXssEscapedRequest(string database)
{
var filename = "Iast.StoredXssEscaped.AspNetCore5." + (IastEnabled ? "IastEnabled" : "IastDisabled");
var url = "/Iast/StoredXssEscaped";
var useMicrosoftDataDb = database == "Microsoft.Data.Sqlite";
var url = $"/Iast/StoredXssEscaped?useMicrosoftDataDb={useMicrosoftDataDb}";
IncludeAllHttpSpans = true;
await TryStartApp();
var agent = Fixture.Agent;
Expand All @@ -259,22 +267,24 @@ public async Task TestIastStoredXssEscapedRequest()
var settings = VerifyHelper.GetSpanVerifierSettings();
settings.AddIastScrubbing();

// Add a scrubber to remove the "?param=<value>" from the a single line
(Regex RegexPattern, string Replacement) scrubber = (new Regex(@"\?param=[^ ]+"), "?param=...,\n");
settings.AddRegexScrubber(scrubber);
// Add a scrubber to remove the useMicrosoftDataDb value
settings.AddRegexScrubber((new Regex(@"useMicrosoftDataDb=(True|False)"), "useMicrosoftDataDb=..."));

await VerifyHelper.VerifySpans(spansFiltered, settings)
.UseFileName(filename)
.DisableRequireUniquePrefix();
}

[SkippableFact]
[SkippableTheory]
[Trait("Category", "ArmUnsupported")]
[Trait("RunOnWindows", "True")]
public async Task TestIastStoredSqliRequest()
[InlineData("System.Data.SQLite")]
[InlineData("Microsoft.Data.Sqlite")]
public async Task TestIastStoredSqliRequest(string database)
{
var filename = "Iast.StoredSqli.AspNetCore5." + (IastEnabled ? "IastEnabled" : "IastDisabled");
var url = "/Iast/StoredSqli";
var useMicrosoftDataDb = database == "Microsoft.Data.Sqlite";
var url = $"/Iast/StoredSqli?useMicrosoftDataDb={useMicrosoftDataDb}";
IncludeAllHttpSpans = true;
await TryStartApp();
var agent = Fixture.Agent;
Expand All @@ -285,6 +295,10 @@ public async Task TestIastStoredSqliRequest()
settings.AddIastScrubbing();
settings.AddRegexScrubber(aspNetCorePathScrubber);
settings.AddRegexScrubber(hashScrubber);

// Add a scrubber to remove the useMicrosoftDataDb value
settings.AddRegexScrubber((new Regex(@"useMicrosoftDataDb=(True|False)"), "useMicrosoftDataDb=..."));

await VerifyHelper.VerifySpans(spansFiltered, settings)
.UseFileName(filename)
.DisableRequireUniquePrefix();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
http.request.headers.host: localhost:00000,
http.route: iast/storedsqli,
http.status_code: 200,
http.url: http://localhost:00000/Iast/StoredSqli,
http.url: http://localhost:00000/Iast/StoredSqli?useMicrosoftDataDb=...,
http.useragent: Mistake Not...,
language: dotnet,
runtime-id: Guid_1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
http.request.headers.host: localhost:00000,
http.route: iast/storedxss,
http.status_code: 200,
http.url: http://localhost:00000/Iast/StoredXss?param=%3Cb%3ERawValue%3C/b%3E,
http.url: http://localhost:00000/Iast/StoredXss?param=%3Cb%3ERawValue%3C/b%3E&useMicrosoftDataDb=...,
http.useragent: Mistake Not...,
language: dotnet,
runtime-id: Guid_1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
http.request.headers.host: localhost:00000,
http.route: iast/storedxssescaped,
http.status_code: 200,
http.url: http://localhost:00000/Iast/StoredXssEscaped,
http.url: http://localhost:00000/Iast/StoredXssEscaped?useMicrosoftDataDb=...,
http.useragent: Mistake Not...,
language: dotnet,
runtime-id: Guid_1,
Expand Down
Loading

0 comments on commit 707de2c

Please sign in to comment.