Clean up EmailInjection.qll and related libraries.

ql/src/experimental/CWE-640/EmailInjection.qhelp

@@ -2,32 +2,30 @@
 <qhelp>
   <overview>
     <p>
-      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.
     </p>
   </overview>
   <recommendation>
     <p>
-      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.
     </p>
   </recommendation>
   <example>
     <p>
-      In the following example snippet, the
-      <code>host</code>
-      field is user controlled.
+      In the following example snippet, the <code>host</code> field is user controlled.
     </p>
     <p>
-      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.
     </p>
     <p>
-      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 @@
       .
     </li>
   </references>
-</qhelp>
+</qhelp>

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 }
   }
 }

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, _) }
   }
 }

ql/test/library-tests/semmle/go/frameworks/Email/MailData.ql

@@ -1,4 +1,4 @@
 import go
 
-from MailData f
+from EmailData f
 select f