bpo-39073: validate Address parts to disallow CRLF #19007

epicfaace · 2020-03-15T01:28:01Z

Validate email.headerregistry.Address to disallow CRLF in address parts (username, domain, display_name)

https://bugs.python.org/issue39073

bitdancer

Thanks for the PR.

Lib/email/headerregistry.py

Lib/test/test_email/test_headerregistry.py

bedevere-bot · 2020-03-16T00:59:33Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

…ddress

epicfaace

I have made the requested changes; please review again

epicfaace · 2020-03-16T13:34:25Z

I have made the requested changes; please review again

bedevere-bot · 2020-03-16T13:34:28Z

Thanks for making the requested changes!

@bitdancer: please review the changes made to this pull request.

bitdancer · 2020-03-16T14:47:21Z

Lib/email/headerregistry.py

+
+        inputs = ''.join(filter(None, (display_name, username, domain, addr_spec)))
+        if '\r' in inputs or '\n' in inputs:
+            raise ValueError("invalid inputs; address parts cannot contain CR / LF")


Hmm. Reading this I think I'd say "arguments" rather than inputs, that aligns better with our typical vocabulary. And how about "CR or LF"?

Lib/test/test_email/test_headerregistry.py

bedevere-bot · 2020-03-16T14:47:24Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

epicfaace · 2020-03-16T14:52:16Z

I have made the requested changes; please review again

bedevere-bot · 2020-03-16T14:52:19Z

Thanks for making the requested changes!

@bitdancer: please review the changes made to this pull request.

bitdancer

Except for the news item text this looks good.

bitdancer · 2020-03-16T17:41:47Z

Misc/NEWS.d/next/Security/2020-03-15-01-28-36.bpo-39073.6Szd3i.rst

@@ -0,0 +1 @@
+Validate email.headerregistry.Address to disallow CRLF in address parts (username, domain, display_name).


I thought I'd already made this comment but I can't find it:

"DIsallow CR or LF in email.headerregistry.Address arguments to guard against header injection attacks."

bedevere-bot · 2020-03-30T00:38:44Z

@bitdancer: Please replace # with GH- in the commit message next time. Thanks!

miss-islington · 2020-03-30T00:38:44Z

Thanks @epicfaace for the PR, and @bitdancer for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7, 3.8.
🐍🍒⛏🤖

Disallow CR or LF in email.headerregistry.Address arguments to guard against header injection attacks. (cherry picked from commit 614f172) Co-authored-by: Ashwin Ramaswami <aramaswamis@gmail.com>

bedevere-bot · 2020-03-30T00:39:13Z

GH-19222 is a backport of this pull request to the 3.8 branch.

Disallow CR or LF in email.headerregistry.Address arguments to guard against header injection attacks. (cherry picked from commit 614f172) Co-authored-by: Ashwin Ramaswami <aramaswamis@gmail.com>

bedevere-bot · 2020-03-30T00:39:23Z

GH-19223 is a backport of this pull request to the 3.7 branch.

bedevere-bot · 2020-03-30T00:39:31Z

GH-19224 is a backport of this pull request to the 3.6 branch.

Disallow CR or LF in email.headerregistry.Address arguments to guard against header injection attacks. (cherry picked from commit 614f172) Co-authored-by: Ashwin Ramaswami <aramaswamis@gmail.com>

Disallow CR or LF in email.headerregistry.Address arguments to guard against header injection attacks. (cherry picked from commit 614f172)

Disallow CR or LF in email.headerregistry.Address arguments to guard against header injection attacks. (cherry picked from commit 614f172) Co-authored-by: Ashwin Ramaswami <aramaswamis@gmail.com> Co-authored-by: Ashwin Ramaswami <aramaswamis@gmail.com>

Disallow CR or LF in email.headerregistry.Address arguments to guard against header injection attacks.

Disallow CR or LF in email.headerregistry.Address arguments to guard against header injection attacks. (cherry picked from commit 614f172)

Disallow CR or LF in email.headerregistry.Address arguments to guard against header injection attacks.

validate address parts

38d0a15

epicfaace requested a review from as a code owner Mar 15, 2020

the-knights-who-say-ni added the CLA signed label Mar 15, 2020

bedevere-bot added the awaiting review label Mar 15, 2020

📜🤖 Added by blurb_it.

d8424e5

bitdancer requested changes Mar 16, 2020

View changes

Lib/email/headerregistry.py Outdated Show resolved Hide resolved

Lib/test/test_email/test_headerregistry.py Outdated Show resolved Hide resolved

bedevere-bot added awaiting changes and removed awaiting review labels Mar 16, 2020

epicfaace added 2 commits Mar 16, 2020

do validation at once, on \r and \n, refactor tests

de6ef8c

Merge branch 'val-address' of github.com:epicfaace/cpython into val-a…

950bd01

…ddress

epicfaace reviewed Mar 16, 2020

View changes

bedevere-bot added awaiting change review and removed awaiting changes labels Mar 16, 2020

bedevere-bot requested a review from bitdancer Mar 16, 2020

rm extra spacing

6cac3a0

bitdancer requested changes Mar 16, 2020

View changes

bedevere-bot removed the awaiting change review label Mar 16, 2020

bedevere-bot added the awaiting changes label Mar 16, 2020

epicfaace added 2 commits Mar 16, 2020

Add display_name test

d10d383

update message

81a0231

bedevere-bot added awaiting change review and removed awaiting changes labels Mar 16, 2020

bedevere-bot requested a review from bitdancer Mar 16, 2020

bitdancer requested changes Mar 16, 2020

View changes

bitdancer approved these changes Mar 30, 2020

View changes

bedevere-bot added awaiting merge and removed awaiting change review labels Mar 30, 2020

bitdancer added needs backport to 3.6 needs backport to 3.7 needs backport to 3.8 type-security labels Mar 30, 2020

bitdancer merged commit 614f172 into python:master Mar 30, 2020
9 checks passed

bedevere-bot removed the awaiting merge label Mar 30, 2020

bedevere-bot removed the needs backport to 3.8 label Mar 30, 2020

bedevere-bot removed the needs backport to 3.7 label Mar 30, 2020

bedevere-bot removed the needs backport to 3.6 label Mar 30, 2020

epicfaace deleted the val-address branch May 27, 2020

vegerot pushed a commit to vegerot/cpython that referenced this issue Jun 10, 2020

bpo-39073: validate Address parts to disallow CRLF (python#19007)

192f60e

Disallow CR or LF in email.headerregistry.Address arguments to guard against header injection attacks.

vegerot pushed a commit to vegerot/cpython that referenced this issue Jun 10, 2020

bpo-39073: validate Address parts to disallow CRLF (python#19007)

a1abeac

Disallow CR or LF in email.headerregistry.Address arguments to guard against header injection attacks.

larryhastings pushed a commit that referenced this issue Jun 12, 2020

bpo-39073: validate Address parts to disallow CRLF (#19007) (#20450)

f91a0b6

Disallow CR or LF in email.headerregistry.Address arguments to guard against header injection attacks. (cherry picked from commit 614f172)

gmelikov pushed a commit to gmelikov/cpython that referenced this issue Aug 22, 2020

bpo-39073: validate Address parts to disallow CRLF (python#19007)

d2db759

Disallow CR or LF in email.headerregistry.Address arguments to guard against header injection attacks.

chrisburr pushed a commit to chrisburr/cpython that referenced this issue Dec 9, 2020

bpo-39073: validate Address parts to disallow CRLF (python#19007)

4a8ecce

Disallow CR or LF in email.headerregistry.Address arguments to guard against header injection attacks.

bpo-39073: validate Address parts to disallow CRLF #19007

bpo-39073: validate Address parts to disallow CRLF #19007

epicfaace commented Mar 15, 2020 •

edited by bedevere-bot

bitdancer left a comment

bedevere-bot commented Mar 16, 2020

epicfaace left a comment

epicfaace commented Mar 16, 2020

bedevere-bot commented Mar 16, 2020

bitdancer Mar 16, 2020

bedevere-bot commented Mar 16, 2020

epicfaace commented Mar 16, 2020

bedevere-bot commented Mar 16, 2020

bitdancer left a comment

bitdancer Mar 16, 2020

bedevere-bot commented Mar 30, 2020

miss-islington commented Mar 30, 2020

bedevere-bot commented Mar 30, 2020

bedevere-bot commented Mar 30, 2020

bedevere-bot commented Mar 30, 2020

		@@ -0,0 +1 @@
		Validate email.headerregistry.Address to disallow CRLF in address parts (username, domain, display_name).

bpo-39073: validate Address parts to disallow CRLF #19007

bpo-39073: validate Address parts to disallow CRLF #19007

Conversation

epicfaace commented Mar 15, 2020 • edited by bedevere-bot

bitdancer left a comment

bedevere-bot commented Mar 16, 2020

epicfaace left a comment

epicfaace commented Mar 16, 2020

bedevere-bot commented Mar 16, 2020

bitdancer Mar 16, 2020

Choose a reason for hiding this comment

bedevere-bot commented Mar 16, 2020

epicfaace commented Mar 16, 2020

bedevere-bot commented Mar 16, 2020

bitdancer left a comment

bitdancer Mar 16, 2020

Choose a reason for hiding this comment

bedevere-bot commented Mar 30, 2020

miss-islington commented Mar 30, 2020

bedevere-bot commented Mar 30, 2020

bedevere-bot commented Mar 30, 2020

bedevere-bot commented Mar 30, 2020

epicfaace commented Mar 15, 2020 •

edited by bedevere-bot