Merge pull request #180 from owen-mc/email-injection

Move email injection query out of experimental folder

change-notes/2020-06-16-email-injection.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The query "Email injection" (`go/email-injection`) has been moved out of the experimental folder. The query detects when untrusted input can be incorporated directly into an email.

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

@@ -4,7 +4,7 @@
     <p>
       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
+      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>
@@ -19,13 +19,13 @@
       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 their own web site. This means the
+      A malicious user can send an HTTP request to the targeted website,
+      but with a Host header that refers to their own website. 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.
+      they trust, but with links leading to a malicious website.
     </p>
     <p>
-      If the email contains a password reset link, and should the victim click
+      If the email contains a password reset link, and the victim clicks
       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.
@@ -38,7 +38,7 @@
   </example>
   <references>
     <li>
-      OWASP
+      OWASP:
       <a href="https://owasp.org/www-community/attacks/Content_Spoofing">Content Spoofing</a>
       .
     </li>

ql/src/Security/CWE-640/EmailInjection.ql

@@ -8,6 +8,7 @@
  * @problem.severity error
  * @tags security
  *       external/cwe/cwe-640
+ * @precision high
  */
 
 import go

ql/test/experimental/CWE-640/EmailInjection.expected

@@ -1,43 +0,0 @@
-edges
-| email.go:24:10:24:17 | selection of Header : Header | email.go:27:56:27:67 | type conversion |
-| email.go:34:21:34:31 | call to Referer : string | email.go:36:57:36:78 | type conversion |
-| email.go:42:21:42:31 | call to Referer : string | email.go:45:3:45:7 | definition of write |
-| email.go:51:21:51:31 | call to Referer : string | email.go:57:46:57:59 | untrustedInput |
-| email.go:51:21:51:31 | call to Referer : string | email.go:58:52:58:65 | untrustedInput |
-| email.go:63:21:63:31 | call to Referer : string | email.go:68:16:68:22 | content |
-| email.go:73:21:73:31 | call to Referer : string | email.go:81:50:81:56 | content |
-| email.go:73:21:73:31 | call to Referer : string | email.go:81:59:81:65 | content |
-| email.go:73:21:73:31 | call to Referer : string | email.go:82:16:82:22 | content |
-| email.go:87:21:87:31 | call to Referer : string | email.go:94:37:94:50 | untrustedInput |
-| email.go:87:21:87:31 | call to Referer : string | email.go:98:16:98:23 | content2 |
-nodes
-| email.go:24:10:24:17 | selection of Header : Header | semmle.label | selection of Header : Header |
-| email.go:27:56:27:67 | type conversion | semmle.label | type conversion |
-| email.go:34:21:34:31 | call to Referer : string | semmle.label | call to Referer : string |
-| email.go:36:57:36:78 | type conversion | semmle.label | type conversion |
-| email.go:42:21:42:31 | call to Referer : string | semmle.label | call to Referer : string |
-| email.go:45:3:45:7 | definition of write | semmle.label | definition of write |
-| email.go:51:21:51:31 | call to Referer : string | semmle.label | call to Referer : string |
-| email.go:57:46:57:59 | untrustedInput | semmle.label | untrustedInput |
-| email.go:58:52:58:65 | untrustedInput | semmle.label | untrustedInput |
-| email.go:63:21:63:31 | call to Referer : string | semmle.label | call to Referer : string |
-| email.go:68:16:68:22 | content | semmle.label | content |
-| email.go:73:21:73:31 | call to Referer : string | semmle.label | call to Referer : string |
-| email.go:81:50:81:56 | content | semmle.label | content |
-| email.go:81:59:81:65 | content | semmle.label | content |
-| email.go:82:16:82:22 | content | semmle.label | content |
-| email.go:87:21:87:31 | call to Referer : string | semmle.label | call to Referer : string |
-| email.go:94:37:94:50 | untrustedInput | semmle.label | untrustedInput |
-| email.go:98:16:98:23 | content2 | semmle.label | content2 |
-#select
-| email.go:27:56:27:67 | type conversion | email.go:24:10:24:17 | selection of Header : Header | email.go:27:56:27:67 | type conversion | Email content may contain $@. | email.go:24:10:24:17 | selection of Header | untrusted input |
-| email.go:36:57:36:78 | type conversion | email.go:34:21:34:31 | call to Referer : string | email.go:36:57:36:78 | type conversion | Email content may contain $@. | email.go:34:21:34:31 | call to Referer | untrusted input |
-| email.go:45:3:45:7 | definition of write | email.go:42:21:42:31 | call to Referer : string | email.go:45:3:45:7 | definition of write | Email content may contain $@. | email.go:42:21:42:31 | call to Referer | untrusted input |
-| email.go:57:46:57:59 | untrustedInput | email.go:51:21:51:31 | call to Referer : string | email.go:57:46:57:59 | untrustedInput | Email content may contain $@. | email.go:51:21:51:31 | call to Referer | untrusted input |
-| email.go:58:52:58:65 | untrustedInput | email.go:51:21:51:31 | call to Referer : string | email.go:58:52:58:65 | untrustedInput | Email content may contain $@. | email.go:51:21:51:31 | call to Referer | untrusted input |
-| email.go:68:16:68:22 | content | email.go:63:21:63:31 | call to Referer : string | email.go:68:16:68:22 | content | Email content may contain $@. | email.go:63:21:63:31 | call to Referer | untrusted input |
-| email.go:81:50:81:56 | content | email.go:73:21:73:31 | call to Referer : string | email.go:81:50:81:56 | content | Email content may contain $@. | email.go:73:21:73:31 | call to Referer | untrusted input |
-| email.go:81:59:81:65 | content | email.go:73:21:73:31 | call to Referer : string | email.go:81:59:81:65 | content | Email content may contain $@. | email.go:73:21:73:31 | call to Referer | untrusted input |
-| email.go:82:16:82:22 | content | email.go:73:21:73:31 | call to Referer : string | email.go:82:16:82:22 | content | Email content may contain $@. | email.go:73:21:73:31 | call to Referer | untrusted input |
-| email.go:94:37:94:50 | untrustedInput | email.go:87:21:87:31 | call to Referer : string | email.go:94:37:94:50 | untrustedInput | Email content may contain $@. | email.go:87:21:87:31 | call to Referer | untrusted input |
-| email.go:98:16:98:23 | content2 | email.go:87:21:87:31 | call to Referer : string | email.go:98:16:98:23 | content2 | Email content may contain $@. | email.go:87:21:87:31 | call to Referer | untrusted input |

ql/test/experimental/CWE-640/EmailInjection.qlref

@@ -1 +0,0 @@
-experimental/CWE-640/EmailInjection.ql

ql/test/query-tests/Security/CWE-640/EmailBad.go

@@ -0,0 +1,13 @@
+package main
+
+import (
+	"net/http"
+	"net/smtp"
+)
+
+func mail(w http.ResponseWriter, r *http.Request) {
+	host := r.Header.Get("Host")
+	token := backend.getUserSecretResetToken(email)
+	body := "Click to reset password: " + host + "/" + token
+	smtp.SendMail("test.test", nil, "from@from.com", nil, []byte(body))
+}

ql/test/query-tests/Security/CWE-640/EmailGood.go

@@ -0,0 +1,13 @@
+package main
+
+import (
+	"net/http"
+	"net/smtp"
+)
+
+func mailGood(w http.ResponseWriter, r *http.Request) {
+	host := config["Host"]
+	token := backend.getUserSecretResetToken(email)
+	body := "Click to reset password: " + host + "/" + token
+	smtp.SendMail("test.test", nil, "from@from.com", nil, []byte(body))
+}

ql/test/query-tests/Security/CWE-640/EmailInjection.expected

@@ -0,0 +1,43 @@
+edges
+| EmailBad.go:9:10:9:17 | selection of Header : Header | EmailBad.go:12:56:12:67 | type conversion |
+| main.go:29:21:29:31 | call to Referer : string | main.go:31:57:31:78 | type conversion |
+| main.go:37:21:37:31 | call to Referer : string | main.go:40:3:40:7 | definition of write |
+| main.go:46:21:46:31 | call to Referer : string | main.go:52:46:52:59 | untrustedInput |
+| main.go:46:21:46:31 | call to Referer : string | main.go:53:52:53:65 | untrustedInput |
+| main.go:58:21:58:31 | call to Referer : string | main.go:63:16:63:22 | content |
+| main.go:68:21:68:31 | call to Referer : string | main.go:76:50:76:56 | content |
+| main.go:68:21:68:31 | call to Referer : string | main.go:76:59:76:65 | content |
+| main.go:68:21:68:31 | call to Referer : string | main.go:77:16:77:22 | content |
+| main.go:82:21:82:31 | call to Referer : string | main.go:89:37:89:50 | untrustedInput |
+| main.go:82:21:82:31 | call to Referer : string | main.go:93:16:93:23 | content2 |
+nodes
+| EmailBad.go:9:10:9:17 | selection of Header : Header | semmle.label | selection of Header : Header |
+| EmailBad.go:12:56:12:67 | type conversion | semmle.label | type conversion |
+| main.go:29:21:29:31 | call to Referer : string | semmle.label | call to Referer : string |
+| main.go:31:57:31:78 | type conversion | semmle.label | type conversion |
+| main.go:37:21:37:31 | call to Referer : string | semmle.label | call to Referer : string |
+| main.go:40:3:40:7 | definition of write | semmle.label | definition of write |
+| main.go:46:21:46:31 | call to Referer : string | semmle.label | call to Referer : string |
+| main.go:52:46:52:59 | untrustedInput | semmle.label | untrustedInput |
+| main.go:53:52:53:65 | untrustedInput | semmle.label | untrustedInput |
+| main.go:58:21:58:31 | call to Referer : string | semmle.label | call to Referer : string |
+| main.go:63:16:63:22 | content | semmle.label | content |
+| main.go:68:21:68:31 | call to Referer : string | semmle.label | call to Referer : string |
+| main.go:76:50:76:56 | content | semmle.label | content |
+| main.go:76:59:76:65 | content | semmle.label | content |
+| main.go:77:16:77:22 | content | semmle.label | content |
+| main.go:82:21:82:31 | call to Referer : string | semmle.label | call to Referer : string |
+| main.go:89:37:89:50 | untrustedInput | semmle.label | untrustedInput |
+| main.go:93:16:93:23 | content2 | semmle.label | content2 |
+#select
+| EmailBad.go:12:56:12:67 | type conversion | EmailBad.go:9:10:9:17 | selection of Header : Header | EmailBad.go:12:56:12:67 | type conversion | Email content may contain $@. | EmailBad.go:9:10:9:17 | selection of Header | untrusted input |
+| main.go:31:57:31:78 | type conversion | main.go:29:21:29:31 | call to Referer : string | main.go:31:57:31:78 | type conversion | Email content may contain $@. | main.go:29:21:29:31 | call to Referer | untrusted input |
+| main.go:40:3:40:7 | definition of write | main.go:37:21:37:31 | call to Referer : string | main.go:40:3:40:7 | definition of write | Email content may contain $@. | main.go:37:21:37:31 | call to Referer | untrusted input |
+| main.go:52:46:52:59 | untrustedInput | main.go:46:21:46:31 | call to Referer : string | main.go:52:46:52:59 | untrustedInput | Email content may contain $@. | main.go:46:21:46:31 | call to Referer | untrusted input |
+| main.go:53:52:53:65 | untrustedInput | main.go:46:21:46:31 | call to Referer : string | main.go:53:52:53:65 | untrustedInput | Email content may contain $@. | main.go:46:21:46:31 | call to Referer | untrusted input |
+| main.go:63:16:63:22 | content | main.go:58:21:58:31 | call to Referer : string | main.go:63:16:63:22 | content | Email content may contain $@. | main.go:58:21:58:31 | call to Referer | untrusted input |
+| main.go:76:50:76:56 | content | main.go:68:21:68:31 | call to Referer : string | main.go:76:50:76:56 | content | Email content may contain $@. | main.go:68:21:68:31 | call to Referer | untrusted input |
+| main.go:76:59:76:65 | content | main.go:68:21:68:31 | call to Referer : string | main.go:76:59:76:65 | content | Email content may contain $@. | main.go:68:21:68:31 | call to Referer | untrusted input |
+| main.go:77:16:77:22 | content | main.go:68:21:68:31 | call to Referer : string | main.go:77:16:77:22 | content | Email content may contain $@. | main.go:68:21:68:31 | call to Referer | untrusted input |
+| main.go:89:37:89:50 | untrustedInput | main.go:82:21:82:31 | call to Referer : string | main.go:89:37:89:50 | untrustedInput | Email content may contain $@. | main.go:82:21:82:31 | call to Referer | untrusted input |
+| main.go:93:16:93:23 | content2 | main.go:82:21:82:31 | call to Referer : string | main.go:93:16:93:23 | content2 | Email content may contain $@. | main.go:82:21:82:31 | call to Referer | untrusted input |

ql/test/query-tests/Security/CWE-640/EmailInjection.qlref

@@ -0,0 +1 @@
+Security/CWE-640/EmailInjection.ql

ql/test/query-tests/Security/CWE-640/main.go

@@ -3,6 +3,9 @@ package main
 //go:generate depstubber -vendor github.com/sendgrid/sendgrid-go/helpers/mail "" NewEmail,NewSingleEmail,NewContent,NewV3Mail,NewV3MailInit
 
 import (
+	"crypto/hmac"
+	"crypto/sha256"
+	"encoding/base64"
 	"io"
 	"log"
 	"net/http"
@@ -11,23 +14,15 @@ import (
 	sendgrid "github.com/sendgrid/sendgrid-go/helpers/mail"
 )
 
-// OK
-func mailGood(w http.ResponseWriter, r *http.Request) {
-	host := config["Host"]
-	token := backend.getUserSecretResetToken(email)
-	body := "Click to reset password: " + host + "/" + token
-	smtp.SendMail("test.test", nil, "from@from.com", nil, []byte(body))
-}
-
-// Not OK
-func mail(w http.ResponseWriter, r *http.Request) {
-	host := r.Header.Get("Host")
-	token := backend.getUserSecretResetToken(email)
-	body := "Click to reset password: " + host + "/" + token
-	smtp.SendMail("test.test", nil, "from@from.com", nil, []byte(body))
-}
-
 func main() {
+	var w http.ResponseWriter
+	var r *http.Request
+
+	// Not OK
+	mail(w, r)
+
+	// OK
+	mailGood(w, r)
 
 	// Not OK
 	http.HandleFunc("/ex0", func(w http.ResponseWriter, r *http.Request) {
@@ -98,6 +93,19 @@ func main() {
 		v.AddContent(content2)
 	})
 
+	// OK
+	http.HandleFunc("/ex6", func(w http.ResponseWriter, r *http.Request) {
+		untrustedInput := r.Referer()
+
+		sha256 := sha256.New
+		appsecret := "appid"
+		hash := hmac.New(sha256, []byte(appsecret))
+		hash.Write([]byte(untrustedInput))
+		signature := base64.StdEncoding.EncodeToString(hash.Sum(nil))
+
+		smtp.SendMail("test.test", nil, "from@from.com", nil, []byte(signature))
+	})
+
 	log.Println(http.ListenAndServe(":80", nil))
 
 }