diff --git a/change-notes/2020-06-16-email-injection.md b/change-notes/2020-06-16-email-injection.md new file mode 100644 index 00000000000..63abd57f2d6 --- /dev/null +++ b/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. diff --git a/ql/src/experimental/CWE-640/EmailBad.go b/ql/src/Security/CWE-640/EmailBad.go similarity index 100% rename from ql/src/experimental/CWE-640/EmailBad.go rename to ql/src/Security/CWE-640/EmailBad.go diff --git a/ql/src/experimental/CWE-640/EmailGood.go b/ql/src/Security/CWE-640/EmailGood.go similarity index 100% rename from ql/src/experimental/CWE-640/EmailGood.go rename to ql/src/Security/CWE-640/EmailGood.go diff --git a/ql/src/experimental/CWE-640/EmailInjection.qhelp b/ql/src/Security/CWE-640/EmailInjection.qhelp similarity index 81% rename from ql/src/experimental/CWE-640/EmailInjection.qhelp rename to ql/src/Security/CWE-640/EmailInjection.qhelp index f3ac1dc4bbe..1da839cedb8 100644 --- a/ql/src/experimental/CWE-640/EmailInjection.qhelp +++ b/ql/src/Security/CWE-640/EmailInjection.qhelp @@ -4,7 +4,7 @@

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.

@@ -19,13 +19,13 @@ 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 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.

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

  • - OWASP + OWASP: Content Spoofing .
    • diff --git a/ql/src/experimental/CWE-640/EmailInjection.ql b/ql/src/Security/CWE-640/EmailInjection.ql similarity index 97% rename from ql/src/experimental/CWE-640/EmailInjection.ql rename to ql/src/Security/CWE-640/EmailInjection.ql index 7cf11fa1a3e..1ae4d751342 100644 --- a/ql/src/experimental/CWE-640/EmailInjection.ql +++ b/ql/src/Security/CWE-640/EmailInjection.ql @@ -8,6 +8,7 @@ * @problem.severity error * @tags security * external/cwe/cwe-640 + * @precision high */ import go diff --git a/ql/src/experimental/CWE-640/EmailInjection.qll b/ql/src/Security/CWE-640/EmailInjection.qll similarity index 100% rename from ql/src/experimental/CWE-640/EmailInjection.qll rename to ql/src/Security/CWE-640/EmailInjection.qll diff --git a/ql/src/experimental/CWE-640/EmailInjectionCustomizations.qll b/ql/src/Security/CWE-640/EmailInjectionCustomizations.qll similarity index 100% rename from ql/src/experimental/CWE-640/EmailInjectionCustomizations.qll rename to ql/src/Security/CWE-640/EmailInjectionCustomizations.qll diff --git a/ql/test/experimental/CWE-640/EmailInjection.expected b/ql/test/experimental/CWE-640/EmailInjection.expected deleted file mode 100644 index 37a3af5ae75..00000000000 --- a/ql/test/experimental/CWE-640/EmailInjection.expected +++ /dev/null @@ -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 | diff --git a/ql/test/experimental/CWE-640/EmailInjection.qlref b/ql/test/experimental/CWE-640/EmailInjection.qlref deleted file mode 100644 index a6d8abad1c9..00000000000 --- a/ql/test/experimental/CWE-640/EmailInjection.qlref +++ /dev/null @@ -1 +0,0 @@ -experimental/CWE-640/EmailInjection.ql \ No newline at end of file diff --git a/ql/test/query-tests/Security/CWE-640/EmailBad.go b/ql/test/query-tests/Security/CWE-640/EmailBad.go new file mode 100644 index 00000000000..aab8467b340 --- /dev/null +++ b/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)) +} diff --git a/ql/test/query-tests/Security/CWE-640/EmailGood.go b/ql/test/query-tests/Security/CWE-640/EmailGood.go new file mode 100644 index 00000000000..5a85e543540 --- /dev/null +++ b/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)) +} diff --git a/ql/test/query-tests/Security/CWE-640/EmailInjection.expected b/ql/test/query-tests/Security/CWE-640/EmailInjection.expected new file mode 100644 index 00000000000..6e84a0466cd --- /dev/null +++ b/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 | diff --git a/ql/test/query-tests/Security/CWE-640/EmailInjection.qlref b/ql/test/query-tests/Security/CWE-640/EmailInjection.qlref new file mode 100644 index 00000000000..98862a2de38 --- /dev/null +++ b/ql/test/query-tests/Security/CWE-640/EmailInjection.qlref @@ -0,0 +1 @@ +Security/CWE-640/EmailInjection.ql \ No newline at end of file diff --git a/ql/test/experimental/CWE-640/go.mod b/ql/test/query-tests/Security/CWE-640/go.mod similarity index 100% rename from ql/test/experimental/CWE-640/go.mod rename to ql/test/query-tests/Security/CWE-640/go.mod diff --git a/ql/test/experimental/CWE-640/email.go b/ql/test/query-tests/Security/CWE-640/main.go similarity index 81% rename from ql/test/experimental/CWE-640/email.go rename to ql/test/query-tests/Security/CWE-640/main.go index 44a54b59fe7..c6685475a8d 100644 --- a/ql/test/experimental/CWE-640/email.go +++ b/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)) } diff --git a/ql/test/experimental/CWE-640/vendor/github.com/sendgrid/sendgrid-go/helpers/LICENSE b/ql/test/query-tests/Security/CWE-640/vendor/github.com/sendgrid/sendgrid-go/helpers/LICENSE similarity index 100% rename from ql/test/experimental/CWE-640/vendor/github.com/sendgrid/sendgrid-go/helpers/LICENSE rename to ql/test/query-tests/Security/CWE-640/vendor/github.com/sendgrid/sendgrid-go/helpers/LICENSE diff --git a/ql/test/experimental/CWE-640/vendor/github.com/sendgrid/sendgrid-go/helpers/mail/stub.go b/ql/test/query-tests/Security/CWE-640/vendor/github.com/sendgrid/sendgrid-go/helpers/mail/stub.go similarity index 100% rename from ql/test/experimental/CWE-640/vendor/github.com/sendgrid/sendgrid-go/helpers/mail/stub.go rename to ql/test/query-tests/Security/CWE-640/vendor/github.com/sendgrid/sendgrid-go/helpers/mail/stub.go diff --git a/ql/test/experimental/CWE-640/vendor/modules.txt b/ql/test/query-tests/Security/CWE-640/vendor/modules.txt similarity index 100% rename from ql/test/experimental/CWE-640/vendor/modules.txt rename to ql/test/query-tests/Security/CWE-640/vendor/modules.txt