-
Notifications
You must be signed in to change notification settings - Fork 147
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
Changes from all commits
36732ad
4d092f5
d958242
4c5aa8a
16c3d5a
ecdd6cd
9f4600a
e372ad1
acf242b
a1f796a
0db2d9e
b978cdf
6ceded9
144102b
3e35506
47d8a99
7e955a4
2212a41
772455a
6ca9b88
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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); | ||
} |
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>(); | ||
} | ||
catch (Exception ex) | ||
{ | ||
IastModule.LogAspectException(ex, $"{nameof(MailkitAspect)}.{nameof(ConvertToMimekit)} (DuckCast)"); | ||
} | ||
|
||
return textPart; | ||
} | ||
} |
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 | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
{ | ||
|
@@ -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; | ||
break; | ||
case EmailInjectionType.AmazonSimpleEmail: | ||
var sendEmailRequest = mail.DuckCast<ISendEmailRequest>(); | ||
body = sendEmailRequest.Message?.Body?.Html?.Data; | ||
isHtml = !string.IsNullOrEmpty(body); | ||
break; | ||
default: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are we not checking There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
|
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 :idkThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it says:
so I don't know if you expect the proxy to fail, maybe not so it's ok like this 🤔
There was a problem hiding this comment.
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.