From 04a19b71504a0211584cba331a57e2b23a2f7596 Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Mon, 4 May 2020 09:13:23 +0100 Subject: [PATCH] Clean up `EmailInjection.qll` and related libraries. --- .../experimental/CWE-640/EmailInjection.qhelp | 26 ++-- .../CWE-640/EmailInjectionCustomizations.qll | 2 +- ql/src/semmle/go/frameworks/Email.qll | 114 ++++++++---------- .../semmle/go/frameworks/Email/MailData.ql | 2 +- 4 files changed, 63 insertions(+), 81 deletions(-) diff --git a/ql/src/experimental/CWE-640/EmailInjection.qhelp b/ql/src/experimental/CWE-640/EmailInjection.qhelp index b1749fbb488..f3ac1dc4bbe 100644 --- a/ql/src/experimental/CWE-640/EmailInjection.qhelp +++ b/ql/src/experimental/CWE-640/EmailInjection.qhelp @@ -2,32 +2,30 @@

- Using untrusted input to construct an email induces multiple security - vulnerabilities. For instance, inclusion of an untrusted input in a email body - may allow an attacker to conduct Cross Site Scripting (XSS) Attacks. While - inclusion of an HTTP Header in the email body may allow a full account - compromise as shown in the example below. + Using untrusted input to construct an email can cause multiple security + vulnerabilities. For instance, inclusion of an untrusted input in an email body + may allow an attacker to conduct Cross Site Scripting (XSS) attacks, while + inclusion of an HTTP header may allow a full account compromise as shown in the + example below.

- Any data which is passed to an email subject or body must be sanitized before use. + Any data which is passed to an email subject or body must be sanitized before use.

- In the following example snippet, the - host - field is user controlled. + In the following example snippet, the host field is user controlled.

- A malicious user can send an HTTP request to the targeted web site, - but with a Host header that refers to his own web site. This means the - emails will be sent out to potential victims, originating from a server + A malicious user can send an HTTP request to the targeted web site, + but with a Host header that refers to their own web site. This means the + emails will be sent out to potential victims, originating from a server they trust, but with links leading to a malicious web site.

- If the email contains a password reset link, and should the victim click + If the email contains a password reset link, and should the victim click the link, the secret reset token will be leaked to the attacker. Using the leaked token, the attacker can then construct the real reset link and use it to change the victim's password. @@ -45,4 +43,4 @@ . - \ No newline at end of file + diff --git a/ql/src/experimental/CWE-640/EmailInjectionCustomizations.qll b/ql/src/experimental/CWE-640/EmailInjectionCustomizations.qll index 104a5111c59..77e3ad97a3b 100644 --- a/ql/src/experimental/CWE-640/EmailInjectionCustomizations.qll +++ b/ql/src/experimental/CWE-640/EmailInjectionCustomizations.qll @@ -25,6 +25,6 @@ module EmailInjection { * A data-flow node that becomes part of an email considered as a taint sink for email injection. */ class MailDataAsSink extends Sink { - MailDataAsSink() { this instanceof MailData } + MailDataAsSink() { this instanceof EmailData } } } diff --git a/ql/src/semmle/go/frameworks/Email.qll b/ql/src/semmle/go/frameworks/Email.qll index 7041cf86b57..90cbb523e19 100644 --- a/ql/src/semmle/go/frameworks/Email.qll +++ b/ql/src/semmle/go/frameworks/Email.qll @@ -3,44 +3,36 @@ import go /** - * A data-flow node that represents data written to an email. - * Data in this case includes the email headers and the mail body + * A data-flow node that represents data written to an email, either as part + * of the headers or as part of the body. * * Extend this class to refine existing API models. If you want to model new APIs, - * extend `MailDataCall::Range` instead. + * extend `EmailData::Range` instead. */ -class MailData extends DataFlow::Node { - MailDataCall::Range self; +class EmailData extends DataFlow::Node { + EmailData::Range self; - MailData() { this = self.getData() } + EmailData() { this = self } } -/** Provides classes for working with calls which write data to an email. */ -module MailDataCall { +/** Provides classes for working with data that is incorporated into an email. */ +module EmailData { /** - * A data-flow node that represents a call which writes data to an email. - * Data in this case refers to email headers and the mail body + * A data-flow node that represents data which is written to an email, either as part + * of the headers or as part of the body. * + * Extend this class to model new APIs. If you want to refine existing API models, + * extend `EmailData` instead. */ - abstract class Range extends DataFlow::CallNode { - /** Gets data written to an email connection. */ - abstract DataFlow::Node getData(); - } + abstract class Range extends DataFlow::Node { } - /** Get the package name `github.com/sendgrid/sendgrid-go/helpers/mail`. */ - bindingset[result] - private string sendgridMail() { result = "github.com/sendgrid/sendgrid-go/helpers/mail" } - - /** A Client.Data expression string used in an API function of the net/smtp package. */ + /** A data-flow node that is written to an email using the net/smtp package. */ private class SmtpData extends Range { SmtpData() { // func (c *Client) Data() (io.WriteCloser, error) - this.getTarget().(Method).hasQualifiedName("net/smtp", "Client", "Data") - } - - override DataFlow::Node getData() { - exists(DataFlow::CallNode write, DataFlow::Node writer, int i | - this.getResult(0) = writer and + exists(Method data, DataFlow::CallNode write, DataFlow::Node writer, int i | + data.hasQualifiedName("net/smtp", "Client", "Data") and + writer = data.getACall().getResult(0) and ( write.getTarget().hasQualifiedName("fmt", "Fprintf") or @@ -48,32 +40,22 @@ module MailDataCall { ) and writer.getASuccessor*() = write.getArgument(0) and i > 0 and - write.getArgument(i) = result + write.getArgument(i) = this + ) + or + // func SendMail(addr string, a Auth, from string, to []string, msg []byte) error + exists(Function sendMail | + sendMail.hasQualifiedName("net/smtp", "SendMail") and + this = sendMail.getACall().getArgument(4) ) } } - /** A send mail expression string used in an API function of the net/smtp package. */ - private class SmtpSendMail extends Range { - SmtpSendMail() { - // func SendMail(addr string, a Auth, from string, to []string, msg []byte) error - this.getTarget().hasQualifiedName("net/smtp", "SendMail") - } + /** Gets the package name `github.com/sendgrid/sendgrid-go/helpers/mail`. */ + bindingset[result] + private string sendgridMail() { result = "github.com/sendgrid/sendgrid-go/helpers/mail" } - override DataFlow::Node getData() { result = this.getArgument(4) } - } - - /** A call to `NewSingleEmail` API function of the Sendgrid mail package. */ - private class SendGridSingleEmail extends Range { - SendGridSingleEmail() { - // func NewSingleEmail(from *Email, subject string, to *Email, plainTextContent string, htmlContent string) *SGMailV3 - this.getTarget().hasQualifiedName(sendgridMail(), "NewSingleEmail") - } - - override DataFlow::Node getData() { result = this.getArgument([1, 3, 4]) } - } - - /* Gets the value of the `i`-th content parameter of the given `call` */ + /* Gets the value of the `i`th content parameter of the given `call` */ private DataFlow::Node getContent(DataFlow::CallNode call, int i) { exists(DataFlow::CallNode cn, DataFlow::Node content | // func NewContent(contentType string, value string) *Content @@ -84,27 +66,29 @@ module MailDataCall { ) } - /** A call to `NewV3MailInit` API function of the Sendgrid mail package. */ - private class SendGridV3Init extends Range { - SendGridV3Init() { - // func NewV3MailInit(from *Email, subject string, to *Email, content ...*Content) *SGMailV3 - this.getTarget().hasQualifiedName(sendgridMail(), "NewV3MailInit") - } - - override DataFlow::Node getData() { - exists(int i | result = getContent(this, i) and i >= 3) + /** A data-flow node that is written to an email using the sendgrid/sendgrid-go package. */ + private class SendGridSingleEmail extends Range { + SendGridSingleEmail() { + // func NewSingleEmail(from *Email, subject string, to *Email, plainTextContent string, htmlContent string) *SGMailV3 + exists(Function newSingleEmail | + newSingleEmail.hasQualifiedName(sendgridMail(), "NewSingleEmail") and + this = newSingleEmail.getACall().getArgument([1, 3, 4]) + ) + or + // func NewV3MailInit(from *Email, subject string, to *Email, content ...*Content) *SGMailV3 + exists(Function newv3MailInit | + newv3MailInit.hasQualifiedName(sendgridMail(), "NewV3MailInit") + | + this = getContent(newv3MailInit.getACall(), any(int i | i >= 3)) + or + this = newv3MailInit.getACall().getArgument(1) + ) or - result = this.getArgument(1) - } - } - - /** A call to `AddContent` API function of the Sendgrid mail package. */ - private class SendGridAddContent extends Range { - SendGridAddContent() { // func (s *SGMailV3) AddContent(c ...*Content) *SGMailV3 - this.getTarget().(Method).hasQualifiedName(sendgridMail(), "SGMailV3", "AddContent") + exists(Method addContent | + addContent.hasQualifiedName(sendgridMail(), "SGMailV3", "AddContent") and + this = getContent(addContent.getACall(), _) + ) } - - override DataFlow::Node getData() { result = getContent(this, _) } } } diff --git a/ql/test/library-tests/semmle/go/frameworks/Email/MailData.ql b/ql/test/library-tests/semmle/go/frameworks/Email/MailData.ql index 4a630d2a96c..c67ce8753e1 100644 --- a/ql/test/library-tests/semmle/go/frameworks/Email/MailData.ql +++ b/ql/test/library-tests/semmle/go/frameworks/Email/MailData.ql @@ -1,4 +1,4 @@ import go -from MailData f +from EmailData f select f