Comment 69 for bug 1449212

Revision history for this message
Samuel Merritt (torgomatic) wrote :

I think the patch in comment #56 is broken. Here's how I tested this stuff:

Test Step 1: saio$ resetswift; startmain

Test Step 2: client$ swift upload victim setup.py setup.cfg AUTHORS # from Swift source tree; any old crap will do

Test Step 3: client$ swift post hacked -m "temp-url-key:conkey"

Test Step 4: curl "http://192.168.22.144:8080`swift tempurl GET 86400 /v1/AUTH_test/hacked/evil-manifest conkey`" # should 4xx

Test Step 5: client$ swift post -m "temp-url-key:acckey"

Test Step 6: curl "http://192.168.22.144:8080`swift tempurl GET 86400 /v1/AUTH_test/hacked/evil-manifest acckey`" # should 200

Base commit: 5b24b22

With no patch, Test Step 4 received a 200 response with a body that was the concatenation of setup.cfg and setup.py.

With the patch from comment #56 applied, Test Step 4 received a 200 response with a body that was the concatenation of setup.cfg and setup.py. I was expecting a 400-series error.

Upon inspection, the patch from comment #56 was missing the code to copy the environment before removing swift.authorize (swift.proxy.server.Application.handle_request). If I add those two lines back in, then Test Step 4 gets a 401, as expected.

The attached patch here is Clay's patch from comment #56, plus the aforementioned two lines of code. I still get unit test failures with this, though.