possible HTTP header injection

Hmm, I don't think this vector is valid.
Even though you can inject \r\n in webob's PATH_INFO, it's not used as-is in real request headers, it gets quoted before it's put to good use:

>>> a='/%0d%0a'
>>> b=urllib.unquote(a)
>>> r=webob.Request.blank('i will be overridden by env', environ={})
>>> r.environ['PATH_INFO']=b
>>> r.path_info
'/\r\n'
>>> r.url
'http://localhost/%0D%0A'

So you can't inject \r\n in the resulting request ?

If r.path or r.url is used, you might be right (I am not a webob expert though).

But I am not so certain that this is always the case:

[...]
 resp2 = self.GETorHEAD_base(req, _('Object'), partition,
                    self.iter_nodes(partition, nodes, self.app.object_ring),
                    req.path_info, self.app.object_ring.replica_count)
[...]
is using path_info which is passed as is to http_connect().

Additionally, some of the unquote()d strings are quote()d when building a new header
and some are not. Is it possible to somehow affect trans_id which is passed unquoted to
the header?

Looking into http_connect, it urllib.quotes the path before using it:
path = quote('/' + device + '/' + str(partition) + path)

On the headers side, they should definitely be a
This definitely should be audited a bit more completely to make sure all cases are covered.

Oops. I meant:

On the headers side, they should definitely be all quoted before being passed to http_connect, or all be unquoted and have http_connect quote them. So a bit more auditing here can't hurt.

Added openstack auditors at openstack-ossg so that they can spend a few audit cycles on that.

Looks like this could potentially be a big problem. The real question is if the resulting values are being quoted everywhere that they are used. Without going through the rest of the code, it is hard to know if this is something that is exploitable today. A code audit seems in order.

In the long run, we should really change how the values are being unquoted. That seems like the wrong way to go about it as it burdens users of this data to get it right every time.

Considering folks have been using spaces in their object names for quite some time, and I believe \r\n (I know I've seen \r and \n separately, don't remember if together) all of these would normally (meaning non-exploit) would've caused a bunch of 400 Bad Requests, I don't think this is an issue at all.

However, a actual working exploit would be wonderful with this and any security report -- especially if it would only take 10-15 minutes to do.

@gholt: Like I explained in previous comments, there is no clear vulnerability for you to fix here, just an area of the code that could use some extra security auditing, and/or code changes to be sure to always quote/unquote rather than relying on the function caller to do so.

Let's see if the auditors can come up with a specific exploitable case first -- if they can't, we'll open up this bug and potentially strengthen the functions around that in public patches.

Since there has been no activity on this bug report for nearly 5 months, and because Swift has been refactored to remove webob, and because there was never any example exploit code produced, I'm closing this bug.

If there is a demonstrable error, please report it. If not, a bug report of "generally look out for X" is not useful.

Sebastian, Thomas: are you OK with making this bug public ? It's now an openstack-secaudit task, as something that needs more analysis.

Hi Thierry,
I am fine with it.

Thanks,
Thomas

Public now