classification
Title: [security] smtplib multiple CRLF injection
Type: security Stage: resolved
Components: Versions: Python 3.11, Python 3.10, Python 3.9, Python 3.8, Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: barry, christian.heimes, lukasz.langa, martin.ortner, miguendes, miss-islington, ned.deily, r.david.murray
Priority: deferred blocker Keywords: patch

Created on 2021-02-04 13:05 by martin.ortner, last changed 2021-08-30 19:22 by ned.deily. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 25987 merged miguendes, 2021-05-08 10:53
PR 28034 merged miss-islington, 2021-08-29 14:10
PR 28035 merged miss-islington, 2021-08-29 14:11
PR 28036 merged miss-islington, 2021-08-29 14:11
PR 28037 merged miss-islington, 2021-08-29 14:11
PR 28038 merged miss-islington, 2021-08-29 14:11
Messages (17)
msg386481 - (view) Author: Martin Ortner (martin.ortner) Date: 2021-02-04 13:05
// reported via PSRT email (see timeline; last contact: Alex/PSRT)
// external reference: http://consensys.net/diligence/vulnerabilities/private/z5kxjgfmja4offxbrw1miuxwezggajjfswlz9g2hfuh77we5dy727hqy5x9ii43e/

cve: 
vendor: python
vendorUrl: https://www.python.org/
authors: tintinweb
affectedVersions: [at least 3.8.3, <=3.7.8, <=3.6.11, <=3.5.9, <=2.7.18]
vulnClass: CWE-93


# Vulnerability Note

## Summary 

>Python is a programming language that lets you work more quickly and integrate your systems more effectively.

Two CR-LF injection points have been discovered in the Python standard library for `SMTP` interaction (client perspective) named `smtplib` that may allow a malicious user with direct access to `smtplib.SMTP(..., local_hostname, ..)` or `smtplib.SMTP(...).mail(..., options)` to inject a CR-LF control sequence to inject arbitrary `SMTP` commands into the protocol stream.

The root cause of this is likely to be found in the design of the `putcmd(cmd, args)` method, that fails to validate that `cmd` nor `args` contains any protocol control sequences (i.e. `CR-LF`). 

It is recommended to reject or encode `\r\n` in `putcmd()` and enforce that potential multi-line commands call `putcmd()` multiple times to avoid that malicious input breaks the expected context of the method and hence cause unexpected behavior. For reference, the `DATA` command (multi-line) would not be affected by this change as it calls `putcmd()` only once and continues with directly interacting with the socket to submit the body.

## Details

### Description



The root cause of this (and probably also some earlier reported CR-LF injections) is the method `putcmd()` in `lib/smtplib.py`[3]. The method is called by multiple commands and does not validate that neither `cmd` nor `args` contains any `CRLF` sequences.

```python
def putcmd(self, cmd, args=""):
    """Send a command to the server."""
    if args == "":
        str = '%s%s' % (cmd, CRLF)
    else:
        str = '%s %s%s' % (cmd, args, CRLF)
    self.send(str)
```

However, the issue was initially found in `mail(..., options)` [4] which fails to ensure that none of the provided `options` contains `CRLF` characters. The method only ensures that provides mail addresses are quoted, `optionslist` is untouched:

```python
self.putcmd("mail", "FROM:%s%s" % (quoteaddr(sender), optionlist))
```

A similar issue was found with `smtplib.SMTP(...,local_hostname)` (and `helo(name)`, `ehlo(name)`) which may potentially contain `CRLF` sequences and, therefore, can be used to inject `SMTP` commands.

Here's a snipped of `helo` [5]
```python
def helo(self, name=''):
    """SMTP 'helo' command.
    Hostname to send for this command defaults to the FQDN of the local
    host.
    """
    self.putcmd("helo", name or self.local_hostname)
    (code, msg) = self.getreply()
    self.helo_resp = msg
    return (code, msg)
```

We highly recommend, fixing this issue once and for all directly in `putcmd()` and enforce that the interface can only send one command at a time, rejecting arguments that contain `CRLF` sequences or properly encoding them to avoid injection.

## Proof of Concept

1. set-up a local tcp listener `⇒  nc -l 10001`

2. run the following PoC and replay the server part as outline in 3.

```python
import smtplib

server = smtplib.SMTP('localhost', 10001, "hi\nX-INJECTED") # localhostname CRLF injection
server.set_debuglevel(1)
server.sendmail("hi@me.com", "you@me.com", "wazzuuup\nlinetwo")
server.mail("hi@me.com",["X-OPTION\nX-INJECTED-1","X-OPTION2\nX-INJECTED-2"]) # options CRLF injection

```

3. interact with `smtplib`, check for `X-INJECTED`

```
⇒  nc -l 10001                       
nc -l 10001
220 yo
ehlo hi
X-INJECTED
250-AUTH PLAIN
250
mail FROM:<hi@me.com>
250 ok
rcpt TO:<you@me.com>
250 ok
data
354 End data with <CR><LF>.<CR><LF> 
wazzuuup
linetwo
.
250 ok
mail FROM:<hi@me.com> X-OPTION
X-INJECTED-1 X-OPTION2
X-INJECTED-2
250 ok
quit
250 ok
```

### Proposed Fix

* enforce that `putcmd` emits exactly one command at a time and encode `\n -> \\n`.

```diff
diff --git a/Lib/smtplib.py b/Lib/smtplib.py
index e2dbbbc..9c16e7d 100755
--- a/Lib/smtplib.py
+++ b/Lib/smtplib.py
@@ -365,10 +365,10 @@ class SMTP:
     def putcmd(self, cmd, args=""):
         """Send a command to the server."""
         if args == "":
-            str = '%s%s' % (cmd, CRLF)
+            str = cmd
         else:
-            str = '%s %s%s' % (cmd, args, CRLF)
-        self.send(str)
+            str = '%s %s' % (cmd, args)
+        self.send('%s%s' % (str.replace('\n','\\n'), CRLF))
```

## Vendor Response

Vendor response: gone silent

### Timeline


```
JUL/02/2020 - contact psrt; provided details, PoC, proposed patch
JUL/04/2020 - confirmed that vulnerability note was received
SEP/10/2020 - requested status update.
```

## References

* [1] https://www.python.org/
* [2] https://www.python.org/downloads/
* [3] https://github.com/python/cpython/blob/1da648aceb2496c672aff82ba37ee071ac6054ac/Lib/smtplib.py#L365-L371
* [4] https://github.com/python/cpython/blob/1da648aceb2496c672aff82ba37ee071ac6054ac/Lib/smtplib.py#L520
* [5] https://github.com/python/cpython/blob/1da648aceb2496c672aff82ba37ee071ac6054ac/Lib/smtplib.py#L428-L445
msg390048 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-04-02 09:35
Deferred the blocker to a regular release due to lack of activity in time for the current expedited releases.
msg393206 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2021-05-07 19:14
Still in "deferred blocker" status awaiting a PR from someone
msg393208 - (view) Author: Miguel Brito (miguendes) * Date: 2021-05-07 19:27
If there's no one working on it I'd be happy to prepare a fix.
msg393209 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2021-05-07 19:32
There is no sign of anyone currently working on it, so please feel free to dig in!
msg394160 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2021-05-21 21:32
Thanks for the PR!  Can someone from the email team take a look at it, please?
msg397421 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2021-07-13 16:01
This bug report starts with "a malicious user with direct access to `smtplib.SMTP(..., local_hostname, ..)", which is a senseless supposition.  Anyone with "access to" the SMTP object could just as well be talking directly to the SMTP server and do anything they want that SMTP itself allows.

The concern here is that data a program might obtain *from unsanitized user input* could be used to do header injection.  The "proof of concept" does not address this at all.  We'd need to see a scenario under which data that could reasonably be derived from user input ends up being passed as arguments to an smtplib method that calls putcmd with arguments.

So, I would rate this as *very* low impact issue, unless someone has an *actual example* of code using smtplib that passes user input through to smtplib commands in an exploitable way.

That said, it is perfectly reasonable to be proactive here and prevent scenarios we haven't yet thought of, by doing as recommended (and a bit more) by raising a ValueError if 'args' in the putcmd call contain either \n or \r characters.  I don't think we need to check 'cmd', because I can't see any scenario in which the SMTP command would be derived from user input.  If you want to be *really* paranoid you could check cmd too, and since it will always be a short string the additional performance impact will be minor.
msg397423 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2021-07-13 16:05
s/header injection/command injection/
msg397433 - (view) Author: Martin Ortner (martin.ortner) Date: 2021-07-13 17:36
> This bug report starts with "a malicious user with direct access to `smtplib.SMTP(..., local_hostname, ..)", which is a senseless supposition.  Anyone with "access to" the SMTP object could just as well be talking directly to the SMTP server and do anything they want that SMTP itself allows.

Let's not argue about the phrasing and settle on the fact that I am not a native English speaker which might be the root cause of the confusion. The core of the issue is that this *unexpected side-effect* may be security-relevant. Fixing it probably takes less time than arguing about phrasing, severity, or spending time describing exploitation scenarios for a general-purpose library that should protect the underlying protocol from injections. 


Be kind, I come in peace.
msg397806 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2021-07-19 15:27
My apologies, I did not think about the possibility of an English issue.  I was reacting to the "security report speak", which I find often makes a security issue sound worse than it is :)  Thank you for reporting this problem, and I do think we should fix it.

My posting was directed at the severity of the issue, since it was potentially holding up a release.  My point about the example is that without an example of code that could reasonably be expected to use user input in a call that could inject newlines, we can treat this as a low priority issue.  If we had a proposed example of such code, then the priority would be higher.  If it was an example of such code "in the wild", then it would be quite high :)

The reason I'm saying we should have an example in order to consider it higher priority is that I cannot see *any* likelihood that this would be a problem in practice.  Let me explain.

putcmd is an *internal* interface.  If we look at the commands that call putcmd or docmd, the only ones that pass extra data that aren't pretty obviously safe (ie: not clearly sanitized data) are rcpt and mail[*].  In both cases the item of concern is optionslist.  optionslist is a list of *SMTP server options*.  This is not data that is reasonably taken from user input, it is data provided *by the programmer*.

[*] I did double check to make sure that email.utils.parseaddr sanitizes both \r and \r, just to be sure :)

Therefore this is *not* a significant security issue.  But as I said, we should take the "defense in depth" approach and apply the check in putcmd as you recommend.  I just don't think it needs to hold up a release.
msg400535 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-08-29 14:10
New changeset 0897253f426068ea6a6fbe0ada01689af9ef1019 by Miguel Brito in branch 'main':
bpo-43124: Fix smtplib multiple CRLF injection (GH-25987)
https://github.com/python/cpython/commit/0897253f426068ea6a6fbe0ada01689af9ef1019
msg400540 - (view) Author: miss-islington (miss-islington) Date: 2021-08-29 14:43
New changeset 9e6c317ab133cd8fa48d5ecd8568314ef2e98634 by Miss Islington (bot) in branch '3.10':
bpo-43124: Fix smtplib multiple CRLF injection (GH-25987)
https://github.com/python/cpython/commit/9e6c317ab133cd8fa48d5ecd8568314ef2e98634
msg400541 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-08-29 14:45
New changeset 24416e419194f11b639146c0d8bed9df315aca5a by Miss Islington (bot) in branch '3.9':
bpo-43124: Fix smtplib multiple CRLF injection (GH-25987) (GH-28035)
https://github.com/python/cpython/commit/24416e419194f11b639146c0d8bed9df315aca5a
msg400543 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-08-29 15:04
New changeset b93aea4c7e4553950daa5d47c3ef2dc8a9c4edff by Miss Islington (bot) in branch '3.8':
[3.8] bpo-43124: Fix smtplib multiple CRLF injection (GH-25987) (GH-28036)
https://github.com/python/cpython/commit/b93aea4c7e4553950daa5d47c3ef2dc8a9c4edff
msg400546 - (view) Author: Łukasz Langa (lukasz.langa) * (Python committer) Date: 2021-08-29 15:09
Thanks, Martin! ✨ 🍰 ✨
msg400661 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2021-08-30 19:16
New changeset d2cc04cd3024869101e894f73307944d98d187c8 by Miss Islington (bot) in branch '3.7':
[3.7] bpo-43124: Fix smtplib multiple CRLF injection (GH-25987) (GH-28037)
https://github.com/python/cpython/commit/d2cc04cd3024869101e894f73307944d98d187c8
msg400663 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2021-08-30 19:22
New changeset 29d97d17fb7adab3b0df9e178b73f70292d1cf64 by Miss Islington (bot) in branch '3.6':
[3.6] bpo-43124: Fix smtplib multiple CRLF injection (GH-25987) (GH-28038)
https://github.com/python/cpython/commit/29d97d17fb7adab3b0df9e178b73f70292d1cf64
History
Date User Action Args
2021-08-30 19:22:04ned.deilysetmessages: + msg400663
2021-08-30 19:16:31ned.deilysetmessages: + msg400661
2021-08-29 15:09:19lukasz.langasetstatus: open -> closed
resolution: fixed
messages: + msg400546

stage: patch review -> resolved
2021-08-29 15:04:27lukasz.langasetmessages: + msg400543
2021-08-29 14:45:29lukasz.langasetmessages: + msg400541
2021-08-29 14:43:46miss-islingtonsetmessages: + msg400540
2021-08-29 14:11:20miss-islingtonsetpull_requests: + pull_request26483
2021-08-29 14:11:15miss-islingtonsetpull_requests: + pull_request26482
2021-08-29 14:11:08miss-islingtonsetpull_requests: + pull_request26481
2021-08-29 14:11:02miss-islingtonsetpull_requests: + pull_request26480
2021-08-29 14:10:57miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request26479
2021-08-29 14:10:53lukasz.langasetmessages: + msg400535
2021-07-19 15:27:39r.david.murraysetmessages: + msg397806
2021-07-13 17:36:33martin.ortnersetmessages: + msg397433
2021-07-13 16:05:41r.david.murraysetmessages: + msg397423
2021-07-13 16:01:31r.david.murraysettype: performance -> security
messages: + msg397421
versions: + Python 3.6, Python 3.7, Python 3.8, Python 3.9, Python 3.10, Python 3.11
2021-07-10 11:46:20pouralialireza2settype: security -> performance
components: - email
versions: - Python 3.6, Python 3.7, Python 3.8, Python 3.9, Python 3.10
2021-05-21 21:32:20ned.deilysetmessages: + msg394160
2021-05-08 10:53:28miguendessetkeywords: + patch
stage: patch review
pull_requests: + pull_request24639
2021-05-07 19:32:34ned.deilysetmessages: + msg393209
2021-05-07 19:27:42miguendessetnosy: + miguendes
messages: + msg393208
2021-05-07 19:14:19ned.deilysetmessages: + msg393206
2021-04-02 09:35:30lukasz.langasetpriority: release blocker -> deferred blocker

messages: + msg390048
2021-03-31 19:58:35christian.heimessetpriority: normal -> release blocker
nosy: + christian.heimes, ned.deily, lukasz.langa
2021-02-04 13:58:08vstinnersettitle: smtplib multiple CRLF injection -> [security] smtplib multiple CRLF injection
2021-02-04 13:05:34martin.ortnercreate