deny injection of custom http headers

[xppt]

Hello!

I've noticed, that aiohttp is simply concatenating server-response (or client-request) header like this, w/o any validation:

    line = status_line + '\r\n' + ''.join(
        [k + ': ' + v + '\r\n' for k, v in headers.items()])

Which may be not okay, if some of the header values were based on user input.

Consider this example:

import aiohttp.web
async def handler(req: aiohttp.web.Request):
    return aiohttp.web.Response(headers={
        'X-Debug-Param': req.query.get('param', ''),
    })
app = aiohttp.web.Application()
app.add_get('/', handler)

This code seems to be fine. Unfortunately it is not, since an attacker can craft urls that will force this handler to return any custom http-headers, or skip some of the existing ones, or broke http payload:

/?param=%0d%0aLocation:%20https://malware.host/  # open redirect
/?param=%0d%0aSet-Cookie:%20...                  # set some cookie
/?param=%0d%0aContent-Length:%2040%0d%0a         # skip next headers

and so on.

I think that aiohttp should raise an exception for any http-reason, header-name or header-value that contains \r or \n characters, instead of breaking http payload silently.

Actually this is what flask/werkzeug do for header-value:

	if u"\n" in value or u"\r" in value:
		raise ValueError(
			"Detected newline in header value.  This is "
			"a potential security problem"
		)

[asvetlov]

Sounds good.
We need a champion for this issue!

[webknjaz]

@xppt this sounds like a security issue. Posting such things on public forums is considered a bad tone security-wise: https://github.com/aio-libs/aiohttp/security/policy. Please consider practicing responsible disclosure next time.

[asvetlov]

Not such bad I think.
I rather consider it a minor issue.
aiohttp is a library. If a code that uses the library doesn't check what is passed to aiohttp -- many very bad things are possible.
But I agree: If we can protect sending malformed HTTP header values -- let's do it.