[issue5871] email.header.Header too lax with embeded newlines

[R. David Murray]

R. David Murray <rdmurray at bitdance.com> added the comment:

I've considered this a bit more deeply, and it turns out to be simpler to fix than I originally thought, assuming the fix is acceptable.

When a message is parsed we obviously wind up with headers that don't have any embedding issues.  So, if we check for embedded headers at message production time and reject them, we can't be breaking any round-trip properties of the package.  The only way for a header malformed in this way to get produced is through it being added to a Message object via one of the header adding APIs.  Further, for this specific issue we are only worried about things that look like header labels that follow a newline and don't have whitespace in front of them.  We don't have to worry about the other RFC restrictions on headers in order to fix this.

I tried a patch that checked at header add time, and while that is potentially workable it got fairly complicated and is a bit fragile (did I get *all* the places a header can be added?)  Instead, the attached patch takes the approach of throwing an error for an embedded header at message serialization time.  The advantage here is that all headers are run through Header.encode on serialization, so there's only one bit of code that needs to be modified to pretty much guarantee that header injection can't happen.

There are code paths for producing individual headers that don't go through encode, but these don't produce complete messages, so it shouldn't be a problem (and may even be a feature).

Barry, do you think this is indeed enough of a security issue that this fix (if acceptable) should be backported to 2.6?

I should note that this patch produces a situation unusual[*] for the email package, where serialization may throw an error (but only on a Message object that has been modified).  Also, I'm reusing HeaderParseError, which may not be optimal, although it does seem at least semi-logical.

[*] Generator currently only throws an error itself in one place, if it encounters a bytes payload for a text Message.  Again, something that can only happen in a modified Message, so this seems analogous.

----------
keywords: +patch
stage: unit test needed -> patch review
versions: +Python 2.7, Python 3.1
Added file: http://bugs.python.org/file20173/header_injection.diff

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue5871>
_______________________________________