0

This is my code:

MailMessage m = new MailMessage(from, to, subject, body);
SmtpClient s = new SmtpClient("...");
s.Send(m);

Only subject and body are user input.

6
  • possible duplicate of Is .NET MailMessage class injection-safe?
    – David Hall
    Jul 9, 2012 at 15:35
  • No, he is specifically asking about the subject and body, while that question is specifically referring to From and To (which use the MailAddress class).
    – tmesser
    Jul 9, 2012 at 15:41
  • @DavidHall It only mentions checking the addresses.
    – ispiro
    Jul 9, 2012 at 15:41
  • Are you setting IsBodyHtml to true? Jul 9, 2012 at 15:47
  • @BrianMains Nope. But I wonder if it would make a difference anyway.
    – ispiro
    Jul 9, 2012 at 15:54

4 Answers 4

2

The code's fine, but it depends on where "subject" and "body" are coming from. If (as you note) they are user-supplied, you want to make sure you are encoding it (HttpServerUtility.HtmlEncode)

1
  • I'm accepting this answer for lack of a better one, since it gave me the de facto solution. But I'm still interested in an answer to whether the HtmlEncode is actually needed.
    – ispiro
    Jul 16, 2012 at 13:09
1

You should validate subject and body then.

2
  • The entire point of this question is asking whether or not they need to be validated (e.g., does the class do it itself).
    – tmesser
    Jul 9, 2012 at 15:39
  • @YYY That's half the question. I would, actually, want to know how to do it if needed.
    – ispiro
    Jul 9, 2012 at 15:40
1

I am attempting to write up a test project to investigate this real fast, but from my look at Reflector and my reading of the documentation, the Subject and Body are strictly treated as System.Strings - this is to the point where you are welcome to explicitly set the encoding on the strings if you want (MailMessage.BodyEncoding).

Unless there is a major bug in how this class is put together, there should be no greater chance of code injection than there would be with any other string; especially if you explicitly set the BodyEncoding to be some manner of plain text, like UTF-8.

EDIT: Alternately, if you really really want to make sure that HTML isn't a part of the body you could use the regex

@"<[^>]*>" 

to naively strip out anything inside a bunch of angle brackets, either with Regex.Replace(regex, string.empty) or Regex.Match and throwing on a return that indicated a match was found.

2
  • Thanks. That's what seems logical, but I don't know how to check that.
    – ispiro
    Jul 9, 2012 at 15:53
  • It is pretty simple. Do you want HTML code to be 'okay' in this setup? If so, set IsBodyHtml to True and the text encoding to something that a normal HTML parser could understand. If not, set IsBodyHtml to false and use a text encoding that supports everything you will expect to get from whatever you're pulling your data from. If it's just English letters, UTF-8 should be fine. If you expect to get some other languages, Unicode may be more appropriate.
    – tmesser
    Jul 9, 2012 at 15:56
0

From the link @DavidHall posted of Is .NET MailMessage class injection-safe?, @Slaks mentions that to/from is validated, but your body content is not. So you would need to validate the subject and body.

Make sure you encode the subject and body before sending it to the user; that should be enough to handle in most scenarios.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.