Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Insecure way to handle json requests #2174

Closed
fduxiao opened this issue Aug 7, 2017 · 7 comments · Fixed by #3889
Closed

Insecure way to handle json requests #2174

fduxiao opened this issue Aug 7, 2017 · 7 comments · Fixed by #3889
Milestone

Comments

@fduxiao
Copy link

fduxiao commented Aug 7, 2017

Long story short

BaseRequest.json coroutine returns request body directly, whose Content-Type should have been checked.

Expected behaviour

Content-Type should be ensured to be application/json so that one can make sure it is sent by ajax, since there are always csrf attacks while ajax can not be forged cross site.

Actual behaviour

Content-Type is not checked.

Steps to reproduce

I setup the server using following code.

#!/usr/bin/env python3

from aiohttp import web
import asyncio


app = web.Application()


async def index(request):
    if request.method == 'GET':
        return web.Response(text='Hello Aiohttp!')
    elif request.method == 'POST':
        j = await request.json()
        return web.json_response(j)


def setup_routes(app):
    app.router.add_route('*', '/', index)


if __name__ == '__main__':
    setup_routes(app)
    web.run_app(app, host='127.0.0.1', port=8080)

And I request the server with following command.

# both of the following will work
curl -v -d '{"abc":2}' -H "Content-Type: application/json" http://127.0.0.1:8080
curl -v -d '{"abc":2}' http://127.0.0.1:8080  # This should fail.

Actually flask handles json requests only with Content-Type: application/json

Environment

> uname -a
Darwin bogon 16.7.0 Darwin Kernel Version 16.7.0: Thu Jun 15 17:36:27 PDT 2017; root:xnu-3789.70.16~2/RELEASE_X86_64 x86_64

> python3 --version  # installed with homebrew
Python 3.6.2

> python3 -c 'import aiohttp; print(aiohttp.__version__)'
2.0.7

> brew --version 
Homebrew 1.3.0
Homebrew/homebrew-core (git revision 51fde5; last commit 2017-08-04)
@socketpair
Copy link
Contributor

Please use latest aiohttp. See #1723

@fduxiao
Copy link
Author

fduxiao commented Aug 7, 2017

That's not what I need. I'm not trying to get the request from a server but to handle the request sent to the server. Now the way to handle will cause CSRF attacks. An error should be raised or directly return 405 instead of getting the json result. Please read my code carefully.

@asvetlov
Copy link
Member

asvetlov commented Aug 7, 2017

For preventing CSRF you need much more than just content type check.

@asvetlov asvetlov reopened this Aug 7, 2017
@fduxiao
Copy link
Author

fduxiao commented Aug 7, 2017

But when I want to make a json web service I just need to check the content type. (See here i.e. the only threat comes from XHR-based CSRF attacks) Other frameworks will auto-check the content type so I can feel free to directly use an interface to get the JSON request or I have to stay alert when I'm writing code, and often, which cause me more easily to make mistakes. So is there any way to prevent it?

@asvetlov
Copy link
Member

asvetlov commented Aug 7, 2017

Well, would you make a PR?

@fduxiao
Copy link
Author

fduxiao commented Aug 8, 2017

Yes, I've made the PR. When one tries getting the JSON data, Content-Type is first check and HTTPNotAcceptable error is raised if it is not application/json. One could also switch off the checking or write a handler himself/herself. In comparison with Flask, getting json requests is more sophisticated, and I don't know whether this kind of way you would prefer. Also, Ruby On Rails would automatically detect the type of the request and allows one to render separated results over the same controller corresponding to the type. Maybe you could add this to aiohttp so that one can save a lot of extra code.

@asvetlov asvetlov added this to the 3.0 milestone Oct 19, 2017
@asvetlov asvetlov modified the milestones: 3.0, 3.1 Feb 12, 2018
@asvetlov asvetlov modified the milestones: 3.1, 3.2 Mar 22, 2018
@asvetlov asvetlov modified the milestones: 3.2, 3.3 May 7, 2018
@asvetlov asvetlov modified the milestones: 3.3, 3.5 Oct 18, 2018
@asvetlov
Copy link
Member

asvetlov commented Jul 5, 2019

I think the proper PR should follow client API design:

    async def json(self, *, encoding: str=None,
                   loads: JSONDecoder=DEFAULT_JSON_DECODER,
                   content_type: Optional[str]='application/json') -> Any:
        """Read and decodes JSON response."""
        if self._body is None:
            await self.read()

        if content_type:
            ctype = self.headers.get(hdrs.CONTENT_TYPE, '').lower()
            if not _is_expected_content_type(ctype, content_type):
                raise ContentTypeError(
                    self.request_info,
                    self.history,
                    message=('Attempt to decode JSON with '
                             'unexpected mimetype: %s' % ctype),
                    headers=self.headers)

        if encoding is None:
            encoding = self.get_encoding()

        return loads(self._body.decode(encoding))  # type: ignore

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants