Skip to content

Commit

Permalink
Merged PR 21497: [release/6.0] MSRC 68590 - newlines in domain literals
Browse files Browse the repository at this point in the history
This add validation for embedded newlines in email addresses.
Based on https://dev.azure.com/dnceng/internal/_git/dotnet-runtime/pullrequest/20738
There is opt-in System.Net.Mail.EnableFullDomainLiterals switch to allow previous behavior
  • Loading branch information
Tomas Weinfurt authored and mmitche committed Mar 14, 2022
1 parent 1cb7505 commit be98e88
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ namespace System.ComponentModel.DataAnnotations
AllowMultiple = false)]
public sealed class EmailAddressAttribute : DataTypeAttribute
{
private static bool EnableFullDomainLiterals { get; } =
AppContext.TryGetSwitch("System.Net.AllowFullDomainLiterals", out bool enable) ? enable : false;

public EmailAddressAttribute()
: base(DataType.EmailAddress)
{
Expand All @@ -27,6 +30,11 @@ public override bool IsValid(object? value)
return false;
}

if (!EnableFullDomainLiterals && (valueAsString.Contains('\r') || valueAsString.Contains('\n')))
{
return false;
}

// only return true if there is only 1 '@' character
// and it is neither the first nor the last character
int index = valueAsString.IndexOf('@');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ protected override IEnumerable<TestCase> InvalidValues()
yield return new TestCase(new EmailAddressAttribute(), 0);
yield return new TestCase(new EmailAddressAttribute(), "");
yield return new TestCase(new EmailAddressAttribute(), " \r \t \n" );
yield return new TestCase(new EmailAddressAttribute(), "someName@[\r\n\tsomeDomain]");
yield return new TestCase(new EmailAddressAttribute(), "@someDomain.com");
yield return new TestCase(new EmailAddressAttribute(), "@[email protected]");
yield return new TestCase(new EmailAddressAttribute(), "someName");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ namespace System.Net.Mail
//
public partial class MailAddress
{
private static bool EnableFullDomainLiterals { get; } =
AppContext.TryGetSwitch("System.Net.AllowFullDomainLiterals", out bool enable) ? enable : false;

// These components form an e-mail address when assembled as follows:
// "EncodedDisplayname" <userName@host>
private readonly Encoding _displayNameEncoding;
Expand Down Expand Up @@ -219,6 +222,12 @@ private string GetHost(bool allowUnicode)
throw new SmtpException(SR.Format(SR.SmtpInvalidHostName, Address), argEx);
}
}

if (!EnableFullDomainLiterals && domain.AsSpan().IndexOfAny('\r', '\n') >= 0)
{
throw new SmtpException(SR.Format(SR.SmtpInvalidHostName, Address));
}

return domain;
}

Expand Down
59 changes: 59 additions & 0 deletions src/libraries/System.Net.Mail/tests/Functional/SmtpClientTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,15 @@
// (C) 2006 John Luke
//

using System.Collections.Generic;
using System.Globalization;
using System.IO;
using System.Net.NetworkInformation;
using System.Net.Sockets;
using System.Reflection;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.DotNet.RemoteExecutor;
using Systen.Net.Mail.Tests;
using Xunit;

Expand Down Expand Up @@ -523,5 +527,60 @@ public async Task SendMail_SendQUITOnDispose(bool asyncSend)
quitReceived.Wait(TimeSpan.FromSeconds(30));
Assert.True(quitMessageReceived, "QUIT message not received");
}

[ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
[InlineData("foo@[\r\n bar]")]
[InlineData("foo@[bar\r\n ]")]
[InlineData("foo@[bar\r\n baz]")]
public void MultiLineDomainLiterals_Enabled_Success(string input)
{
RemoteExecutor.Invoke(static (string @input) =>
{
AppContext.SetSwitch("System.Net.AllowFullDomainLiterals", true);

var address = new MailAddress(@input);

// Using address with new line breaks the protocol so we cannot easily use LoopbackSmtpServer
// Instead we call internal method that does the extra validation.
string? host = (string?)typeof(MailAddress).InvokeMember("GetAddress", BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.InvokeMethod, null, address, new object[] { true });
Assert.Equal(input, host);
}, input).Dispose();
}

[Theory]
[MemberData(nameof(SendMail_MultiLineDomainLiterals_Data))]
public async Task SendMail_MultiLineDomainLiterals_Disabled_Throws(string from, string to, bool asyncSend)
{
using var server = new LoopbackSmtpServer();

using SmtpClient client = server.CreateClient();
client.Credentials = new NetworkCredential("Foo", "Bar");

using var msg = new MailMessage(@from, @to, "subject", "body");

await Assert.ThrowsAsync<SmtpException>(async () =>
{
if (asyncSend)
{
await client.SendMailAsync(msg).WaitAsync(TimeSpan.FromSeconds(30));
}
else
{
client.Send(msg);
}
});
}

public static IEnumerable<object[]> SendMail_MultiLineDomainLiterals_Data()
{
foreach (bool async in new[] { true, false })
{
foreach (string address in new[] { "foo@[\r\n bar]", "foo@[bar\r\n ]", "foo@[bar\r\n baz]" })
{
yield return new object[] { address, "[email protected]", async };
yield return new object[] { "[email protected]", address, async };
}
}
}
}
}

0 comments on commit be98e88

Please sign in to comment.