Request handler allows to call internal functions

[Thomas Biege]

Hi,
let me copy-and-paste here what Sebastian Krahmer <email address hidden> founds.

Can you verify the findings of Sebastian please. Thanks a lot.

Sebastian Krahmer 2012-05-07 14:49:29 CEST
The SWIFT object storage from openstack (swift-1.4.8) has the
following issue:

Sebastian Krahmer 2012-05-07 14:52:21 CEST
The way the request handler is called is insecure.

It should only allow PUT/POST etc. but not to call
internal object functions:

[...]

    def __call__(self, env, start_response):
        """WSGI Application entry point for the Swift Object Server."""
        start_time = time.time()
        req = Request(env)
        self.logger.txn_id = req.headers.get('x-trans-id', None)
        if not check_utf8(req.path_info):
            res = HTTPPreconditionFailed(body='Invalid UTF8')
        else:
            try:
                if hasattr(self, req.method):
                    res = getattr(self, req.method)(req)
                else:
                    res = HTTPMethodNotAllowed()

[...]

so as long as the req.method string is an attribute of the object,
it can be called. Should better check to be one of PUT/POST etc.
for safety.

[John Dickinson (notmyname)]

1) Do you have a working exploit for this?

2) My first idea on solving this is to use system used in the proxy server: mark public methods with the public() decorator and deny requests not marked as public. Do you think that would solve this issue?

[Sebastian Krahmer (krahmer-p)]

Martin Vidner added a PoC to our bugzilla that is:

 curl -X__init__
"http://192.168.125.11:6000/device/replica/account/container/object"
(note that the words are literals and do not need to be substituted)

I think the public decorator would solve this. Its used by all the other nova exported
APIs too.

[Thierry Carrez (ttx)]

This looks like two separate issues. Since the discussion in the comments is on the request handler issue, I'll open a separate bug about the pickling.

[Thierry Carrez (ttx)]

The "Insecure loads()" part will be discussed in https://bugs.launchpad.net/swift/+bug/1006414

[Thierry Carrez (ttx)]

Confirmed the issue, though it is limited to methods of the class and I think they would all gracefully fail when called with the request object, so setting Medium. Should definitely be fixed though. In all three effected server.py.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/8034
Committed: http://github.com/openstack/swift/commit/9f5a6bba1a954c6c3009a824dbb4a5ef582304d5
Submitter: Jenkins
Branch: master

commit 9f5a6bba1a954c6c3009a824dbb4a5ef582304d5
Author: Ionuț Arțăriși <email address hidden>
Date: Fri Jun 1 16:39:35 2012 +0200

    only allow methods which implement HTTP verbs to be called remotely

    This fixes 500 server crashes caused by requests such as:

    curl -X__init__ "http://your-swift-object-server:6000/sda1/p/a/c/o"

    Fixes bug 1005903

    Change-Id: I6c0ad39a29e07ce5f46b0fdbd11a53a9a1010a04

[Sebastian Krahmer (krahmer-p)]

Ok, I agree that the impact is not so high, but still something thats worth fixing.

[Thierry Carrez (ttx)]

Opening the bug since the fix was committed.

Ionut: next time please wait for a decision on how we want to coordinate disclosure of the issue before pushing public patches.