Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ASM] Email injection: MimeKit and SimpleEmail #6614

Merged
merged 20 commits into from
Feb 25, 2025
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// <copyright file="AmazonSimpleEmailAspect.cs" company="Datadog">
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>
#nullable enable

using System;
using Datadog.Trace.AppSec;
using Datadog.Trace.Iast.Dataflow;

namespace Datadog.Trace.Iast.Aspects;

/// <summary> Email html injection class aspect </summary>
[AspectClass("AWSSDK.SimpleEmail", AspectType.Sink, VulnerabilityType.EmailHtmlInjection)]
[global::System.ComponentModel.Browsable(false)]
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
public class AmazonSimpleEmailAspect
{
/// <summary>
/// Launches a email html injection vulnerability if the email body is tainted, it's not escaped and the email is html compatible.
/// </summary>
/// <param name="message">the email message that is going to be sent</param>
/// <returns>the MailMessage</returns>
#if NETFRAMEWORK
[AspectMethodInsertBefore("Amazon.SimpleEmail.AmazonSimpleEmailServiceClient::SendEmail(Amazon.SimpleEmail.Model.SendEmailRequest)")]
#endif
[AspectMethodInsertBefore("Amazon.SimpleEmail.AmazonSimpleEmailServiceClient::SendEmailAsync(Amazon.SimpleEmail.Model.SendEmailRequest,System.Threading.CancellationToken)", 1)]
public static object? Send(object? message)
{
try
{
IastModule.OnEmailHtmlInjection(message, EmailInjectionType.AmazonSimpleEmail);
return message;
}
catch (Exception ex)
{
IastModule.LogAspectException(ex, $"{nameof(MailkitAspect)}.{nameof(Send)}");
return message;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// <copyright file="ISendEmailRequest.cs" company="Datadog">
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>

using Datadog.Trace.DuckTyping;

namespace Datadog.Trace.Iast.Aspects;

#nullable enable

internal interface ISendEmailRequest
{
IMessage Message { get; }
}

internal interface IMessage
{
IBody Body { get; }
}

internal interface IBody
{
IHtml Html { get; }
}

internal interface IHtml
{
string Data { get; }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// <copyright file="IMimeKitTextPart.cs" company="Datadog">
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>

using System.Text;
using Datadog.Trace.DuckTyping;

namespace Datadog.Trace.Iast.Aspects;

#nullable enable

internal interface IMimeKitTextPart
{
string Text { get; set; }

bool IsHtml { get; }

void SetText(string charset, string text);

void SetText(Encoding encoding, string text);
}
111 changes: 111 additions & 0 deletions tracer/src/Datadog.Trace/Iast/Aspects/MailKit/MailkitAspect.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
// <copyright file="MailkitAspect.cs" company="Datadog">
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>
#nullable enable

using System;
using System.Text;
using Datadog.Trace.AppSec;
using Datadog.Trace.DuckTyping;
using Datadog.Trace.Iast.Dataflow;

namespace Datadog.Trace.Iast.Aspects;

/// <summary> Email html injection class aspect </summary>
[AspectClass("Mailkit", AspectType.Sink, VulnerabilityType.EmailHtmlInjection)]
[global::System.ComponentModel.Browsable(false)]
[global::System.ComponentModel.EditorBrowsable(global::System.ComponentModel.EditorBrowsableState.Never)]
public class MailkitAspect
{
/// <summary>
/// Launches a email html injection vulnerability if the email body is tainted, it's not escaped and the email is html compatible.
/// We loose the tainted string when setting it to the email body because it's converted to a stream.
/// Therefore, we cannot check at the moment of sending and we need to instrument the body set text methods.
/// </summary>
/// <param name="instance">the email body instance</param>
/// <param name="encoding">the email encoding</param>
/// <param name="bodyText">the email text</param>
[AspectMethodReplace("MimeKit.TextPart::SetText(System.String,System.String)")]
public static void SetText(object instance, string encoding, string bodyText)
#pragma warning disable DD0005
{
IMimeKitTextPart? textPart = ConvertToMimekit(instance);

if (textPart is not null)
{
textPart.SetText(encoding, bodyText);
CheckForVulnerability(bodyText, textPart);
}
}
#pragma warning restore DD0005

/// <summary>
/// Launches a email html injection vulnerability if the email body is tainted, it's not escaped and the email is html compatible.
/// </summary>
/// <param name="instance">the email body</param>
/// <param name="encoding">the email ending</param>
/// <param name="bodyText">the email text</param>
[AspectMethodReplace("MimeKit.TextPart::SetText(System.Text.Encoding,System.String)")]
public static void SetTextSystemTextEncoding(object instance, Encoding encoding, string bodyText)
#pragma warning disable DD0005
{
IMimeKitTextPart? textPart = ConvertToMimekit(instance);

if (textPart is not null)
{
textPart.SetText(encoding, bodyText);
CheckForVulnerability(bodyText, textPart);
}
}
#pragma warning restore DD0005

/// <summary>
/// Launches a email html injection vulnerability if the email body is tainted, it's not escaped and the email is html compatible.
/// </summary>
/// <param name="instance">the email body</param>
/// <param name="bodyText">the email text</param>
[AspectMethodReplace("MimeKit.TextPart::set_Text(System.String)")]
public static void SetTextProperty(object instance, string bodyText)
#pragma warning disable DD0005
{
IMimeKitTextPart? textPart = ConvertToMimekit(instance);

if (textPart is not null)
{
textPart.Text = bodyText;
CheckForVulnerability(bodyText, textPart);
}
}
#pragma warning restore DD0005

private static void CheckForVulnerability(string bodyText, IMimeKitTextPart textPart)
{
try
{
if (textPart.IsHtml)
{
IastModule.OnEmailHtmlInjection(bodyText);
}
}
catch (Exception ex)
{
IastModule.LogAspectException(ex, $"{nameof(MailkitAspect)}.{nameof(CheckForVulnerability)}");
}
}

private static IMimeKitTextPart? ConvertToMimekit(object instance)
{
IMimeKitTextPart? textPart = null;
try
{
textPart = instance.DuckCast<IMimeKitTextPart>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we use TryDuckCast instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are following the same pattern in other aspects such as JsonDocumentAspects or JavaScriptSerializerAspects. I guess that we should probably log an DuckCast Failure/exception as error. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh ok, makes sense to log, I just thought TryDuckCast might have been more performant, and then testing the bool result to log or not :idk

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it says:

If you know that your proxy is correct and that it will sometimes fail (due to the nature of the integration) you should use TryDuckCast over DuckAs to safely try the proxying and to handle failure. If you don't expect the proxying to fail (i.e. you're not accounting for explicitly known scenarios) then favour DuckCast

so I don't know if you expect the proxy to fail, maybe not so it's ok like this 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it should not fail. I have tested different versions of the assembly and the proxy should not change.

}
catch (Exception ex)
{
IastModule.LogAspectException(ex, $"{nameof(MailkitAspect)}.{nameof(ConvertToMimekit)} (DuckCast)");
}

return textPart;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public class SmtpClientAspect
{
try
{
IastModule.OnEmailHtmlInjection(message);
IastModule.OnEmailHtmlInjection(message, EmailInjectionType.SystemNetMail);
return message;
}
catch (Exception ex) when (ex is not BlockException)
Expand Down
15 changes: 15 additions & 0 deletions tracer/src/Datadog.Trace/Iast/EmailInjectionType.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// <copyright file="EmailInjectionType.cs" company="Datadog">
// Unless explicitly stated otherwise all files in this repository are licensed under the Apache 2 License.
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2017 Datadog, Inc.
// </copyright>

#nullable enable

namespace Datadog.Trace.Iast;

internal enum EmailInjectionType
{
AmazonSimpleEmail,
MailKit,
SystemNetMail
}
48 changes: 44 additions & 4 deletions tracer/src/Datadog.Trace/Iast/IastModule.cs
Original file line number Diff line number Diff line change
Expand Up @@ -850,7 +850,25 @@ internal static IastModuleResponse OnXpathInjection(string xpath)
}
}

internal static void OnEmailHtmlInjection(object? message)
internal static void OnEmailHtmlInjection(string text)
{
if (!Iast.Instance.Settings.Enabled)
{
return;
}

OnExecutedSinkTelemetry(IastVulnerabilityType.EmailHtmlInjection);

if (string.IsNullOrEmpty(text))
{
return;
}

// We use the same secure marks as XSS, but excluding db sources
GetScope(text, IntegrationId.EmailHtmlInjection, VulnerabilityTypeUtils.EmailHtmlInjection, OperationNameEmailHtmlInjection, taintValidator: Always, safeSources: _dbSources, exclusionSecureMarks: SecureMarks.Xss);
}

internal static void OnEmailHtmlInjection(object? message, EmailInjectionType type)
{
if (!Iast.Instance.Settings.Enabled)
{
Expand All @@ -864,15 +882,37 @@ internal static void OnEmailHtmlInjection(object? message)
return;
}

var messageDuck = message.DuckCast<IMailMessage>();
ExtractProperties(message, type, out var body, out var isHtml);

if (messageDuck?.IsBodyHtml is not true || string.IsNullOrEmpty(messageDuck.Body))
if (!isHtml || string.IsNullOrEmpty(body))
{
return;
}

// We use the same secure marks as XSS, but excluding db sources
GetScope(messageDuck.Body, IntegrationId.EmailHtmlInjection, VulnerabilityTypeUtils.EmailHtmlInjection, OperationNameEmailHtmlInjection, taintValidator: Always, safeSources: _dbSources, exclusionSecureMarks: SecureMarks.Xss);
GetScope(body!, IntegrationId.EmailHtmlInjection, VulnerabilityTypeUtils.EmailHtmlInjection, OperationNameEmailHtmlInjection, taintValidator: Always, safeSources: _dbSources, exclusionSecureMarks: SecureMarks.Xss);
}

private static void ExtractProperties(object mail, EmailInjectionType type, out string? body, out bool isHtml)
{
switch (type)
{
case EmailInjectionType.SystemNetMail:
var mailMessage = mail.DuckCast<IMailMessage>();
body = mailMessage?.Body;
isHtml = mailMessage?.IsBodyHtml ?? false;
break;
case EmailInjectionType.AmazonSimpleEmail:
var sendEmailRequest = mail.DuckCast<ISendEmailRequest>();
body = sendEmailRequest?.Message?.Body?.Html?.Data;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
body = sendEmailRequest?.Message?.Body?.Html?.Data;
body = sendEmailRequest.Message.Body.Html.Data;

iiuc, seems like we don't need all theses checks, as object mail is not nullable and if instance is not null return type is not null when duck casting per https://github.com/DataDog/dd-trace-dotnet/blob/master/docs/development/DuckTyping.md#2-using-interface-proxies-in-duckcastt-or-in-duck-chained-properties

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right. I will remove the nullability check in mail? Still, Message might be null, so I will keep those nullability checks. I have updated the code. Thanks!

isHtml = !string.IsNullOrEmpty(body);
break;
default:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we not checking MailKit? if not, it will fall to default and log an error, do we still want to consider it as an error, or a debug?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will not fall into default because we instrument Mailkit differently and we don't call this method when using Mailkit. The problem about mailkit is that we loose the tainting after setting the body text, so we cannot instrument the Send methods.

Log.Error("Error while checking for email injection type.");
body = string.Empty;
isHtml = false;
break;
}
}

public static void LogAspectException(Exception ex, string aspectInfo)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,19 @@ int GeneratedDefinitions::InitCallSites(UINT32 enabledCategories, UINT32 platfor
{
std::vector<WCHAR*> callSites =
{
(WCHAR*)WStr("[AspectClass(\"AWSSDK.SimpleEmail\",[None],Sink,[EmailHtmlInjection])] Datadog.Trace.Iast.Aspects.AmazonSimpleEmailAspect 4"),
#if _WIN32
(WCHAR*)WStr(" [AspectMethodInsertBefore(\"Amazon.SimpleEmail.AmazonSimpleEmailServiceClient::SendEmail(Amazon.SimpleEmail.Model.SendEmailRequest)\",\"\",[0],[False],[None],Default,[])] Send(System.Object) 1"),
#endif
(WCHAR*)WStr(" [AspectMethodInsertBefore(\"Amazon.SimpleEmail.AmazonSimpleEmailServiceClient::SendEmailAsync(Amazon.SimpleEmail.Model.SendEmailRequest,System.Threading.CancellationToken)\",\"\",[1],[False],[None],Default,[])] Send(System.Object) 15"),
(WCHAR*)WStr("[AspectClass(\"EntityFramework\",[None],Sink,[SqlInjection])] Datadog.Trace.Iast.Aspects.EntityCommandAspect 12"),
(WCHAR*)WStr(" [AspectMethodInsertBefore(\"System.Data.Entity.Core.EntityClient.EntityCommand::ExecuteReader(System.Data.CommandBehavior)\",\"\",[1],[False],[None],Default,[])] ReviewSqlCommand(System.Object) 15"),
(WCHAR*)WStr(" [AspectMethodInsertBefore(\"System.Data.Entity.Core.EntityClient.EntityCommand::ExecuteReaderAsync(System.Data.CommandBehavior)\",\"\",[1],[False],[None],Default,[])] ReviewSqlCommand(System.Object) 15"),
(WCHAR*)WStr(" [AspectMethodInsertBefore(\"System.Data.Entity.Core.EntityClient.EntityCommand::ExecuteReaderAsync(System.Data.CommandBehavior,System.Threading.CancellationToken)\",\"\",[2],[False],[None],Default,[])] ReviewSqlCommand(System.Object) 15"),
(WCHAR*)WStr("[AspectClass(\"Mailkit\",[None],Sink,[EmailHtmlInjection])] Datadog.Trace.Iast.Aspects.MailkitAspect 4"),
(WCHAR*)WStr(" [AspectMethodReplace(\"MimeKit.TextPart::SetText(System.String,System.String)\",\"\",[0],[False],[None],Default,[])] SetText(System.Object,System.String,System.String) 15"),
(WCHAR*)WStr(" [AspectMethodReplace(\"MimeKit.TextPart::SetText(System.Text.Encoding,System.String)\",\"\",[0],[False],[None],Default,[])] SetTextSystemTextEncoding(System.Object,System.Text.Encoding,System.String) 15"),
(WCHAR*)WStr(" [AspectMethodReplace(\"MimeKit.TextPart::set_Text(System.String)\",\"\",[0],[False],[None],Default,[])] SetTextProperty(System.Object,System.String) 15"),
(WCHAR*)WStr("[AspectClass(\"Microsoft.AspNetCore.Http\",[None],Sink,[UnvalidatedRedirect])] Datadog.Trace.Iast.Aspects.AspNetCore.Http.HttpResponseAspect 4"),
(WCHAR*)WStr(" [AspectMethodInsertBefore(\"Microsoft.AspNetCore.Http.HttpResponse::Redirect(System.String)\",\"\",[0],[False],[None],Default,[])] Redirect(System.String) 15"),
(WCHAR*)WStr(" [AspectMethodInsertBefore(\"Microsoft.AspNetCore.Http.HttpResponse::Redirect(System.String,System.Boolean)\",\"\",[1],[False],[None],Default,[])] Redirect(System.String) 15"),
Expand Down
6 changes: 5 additions & 1 deletion tracer/src/Datadog.Tracer.Native/iast/dataflow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,11 @@ static const WSTRING _fixedAssemblyExcludeFilters[] = {
WStr("Npgsql*"),
WStr("Grpc.Net.Client"),
WStr("Amazon.Runtime*"),
WStr("App.Metrics.Concurrency*"),
WStr("App.Metrics.Concurrency*"),
WStr("AWSSDK.SimpleEmail"),
WStr("AWSSDK.Core"),
WStr("MailKit"),
WStr("MimeKit"),
LastEntry, // Can't have an empty array. This must be the last element
};
static const WSTRING _fixedMethodIncludeFilters[] = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@

<ItemGroup>
<PackageReference Include="Verify.Xunit" Version="14.13.1" />
<PackageReference Include="AWSSDK.SimpleEmail" Version="3.7.402.28" />
<PackageReference Include="MailKit" Version="4.10.0" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@
using System.Threading.Tasks;
using System.Xml;
using System.Xml.XPath;
using Amazon.SimpleEmail;
using Datadog.Trace.Configuration;
using Datadog.Trace.TestHelpers;
using FluentAssertions;
using FluentAssertions.Equivalency.Tracing;
using MimeKit;
using Xunit;
using Xunit.Abstractions;
using DirectoryEntry = System.DirectoryServices.DirectoryEntry;
Expand Down Expand Up @@ -151,6 +153,12 @@ public List<string> GetAspects()
[InlineData(typeof(SmtpClient), "Send", new[] { "Void Send(System.String, System.String, System.String, System.String)" }, true)]
[InlineData(typeof(SmtpClient), "SendAsync", new[] { "Void SendAsync(System.String, System.String, System.String, System.String, System.Object)" }, true)]
[InlineData(typeof(SmtpClient), "SendMailAsync", new[] { "System.Threading.Tasks.Task SendMailAsync(System.String, System.String, System.String, System.String, System.Threading.CancellationToken)", "System.Threading.Tasks.Task SendMailAsync(System.String, System.String, System.String, System.String)" }, true)]
#if NETFRAMEWORK
[InlineData(typeof(AmazonSimpleEmailServiceClient), "SendEmail", null, true)]
#endif
[InlineData(typeof(AmazonSimpleEmailServiceClient), "SendEmailAsync", null, true)]
[InlineData(typeof(TextPart), "SetText", null, false)]
[InlineData(typeof(TextPart), "set_Text", null, false)]
#if NET6_0_OR_GREATER
[InlineData(typeof(DefaultInterpolatedStringHandler), null, new[] { "Void AppendFormatted(System.ReadOnlySpan`1[System.Char], Int32, System.String)" }, true)]
#endif
Expand Down Expand Up @@ -293,6 +301,8 @@ public void TestFileClassMethodsAspectCover()
[InlineData(typeof(Extensions))]
[InlineData(typeof(XPathExpression))]
[InlineData(typeof(SmtpClient), new[] { "System.Net.Mail.SmtpClient::SendMailAsync(System.Net.Mail.MailMessage,System.Threading.CancellationToken)\",\"\",[1],[False],[None],Default,[])]" })]
[InlineData(typeof(AmazonSimpleEmailServiceClient))]
[InlineData(typeof(TextPart))]
[InlineData(typeof(Activator), new string[] { "System.Activator::CreateInstance(System.AppDomain,System.String,System.String)" })]
#if !NETFRAMEWORK
#if NET9_0_OR_GREATER
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@
"vulnerabilities": [
{
"type": "EMAIL_HTML_INJECTION",
"hash": -543813396,
"hash": 799617955,
"location": {
"spanId": XXX,
"path": "Samples.Security.AspNetCore5.Controllers.IastController",
"method": "SendMailAux"
"path": "Samples.Security.AspNetCore5.Helpers.EmailHelper",
"method": "SendEmailSystemLib"
},
"evidence": {
"valueParts": [
Expand Down
Loading
Loading