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

Malformed token raises an uncaught ValueError #246

Closed
vimalloc opened this issue May 13, 2019 · 6 comments
Closed

Malformed token raises an uncaught ValueError #246

vimalloc opened this issue May 13, 2019 · 6 comments
Assignees

Comments

@vimalloc
Copy link
Owner

See https://stackoverflow.com/q/55917908/272689

@vimalloc vimalloc self-assigned this May 13, 2019
@vimalloc
Copy link
Owner Author

vimalloc commented May 13, 2019

We shouldn’t add an error handler directly for the value error to handle this, as that could easily start catching errors unrelated to JWTs. Instead we should catch the value error at the PyJWT decode call, and reraise it as something more specific.

@gk-patel
Copy link

gk-patel commented Jun 7, 2019

Hi @vimalloc,

Firstly, you are maintaining a very nice library here, thank you and others for that.

Is it not possible for you to temporarily catch the error in your library and give a minor bug-fix-release ?
Later, you could implement the full thing with catching value error at the PyJWT decode call.

@vimalloc
Copy link
Owner Author

vimalloc commented Jun 7, 2019

Having a temporary errorhandler that catches ValueErrors is not a viable option, it could make debugging someone’s own code significantly harder. However fixing this up properly won’t be hard to do.

As this doesn’t present a security issue and shouldn’t ever show up during a normal users workflow (they would need to explicitly send in garbage to the API to trigger this), it hasn’t been a priority to work on this; it has been a really busy month for me. I’ll try to find time to tackle this as soon as I can, probably in the next couple weeks. Or if you want to submit a pull request to fix this up, I would be happy to help get that merged 👍

@gk-patel
Copy link

gk-patel commented Jun 11, 2019

Ya. You are right, having a quick-fix could be problematic for debugging. Also, it does not present a security issue, but in my case it does come-up during normal user-workflow.

In my implementation of the frontend, I use the refresh_token to ask the server for a new access_token, only when my current access_token has been indicated as invalid -- i.e. I don't use a timer to regularly fetch new access_tokens, rather I get it when my current access_token is invalided. In order to do this, I would need the correct error code (401 or something similar, but I get 500). The console output is as follows,

raise ExpiredSignatureError('Signature has expired')
jwt.exceptions.ExpiredSignatureError: Signature has expired

I am using version 3.18 with flask-restful.

And about the PR, I feel really bad, but I won't be able to find time for that. Because, unlike you, I don't spend my free time in trying to make the web secure (maybe I should), but rather I organize events and so on. Sorry buddy, but I do appreciate your motivation and work. Thanks again, and sorry for the delayed reply.

@vimalloc
Copy link
Owner Author

vimalloc commented Jun 11, 2019

An expired token will not hit this error, that is already handled here: https://github.com/vimalloc/flask-jwt-extended/blob/master/flask_jwt_extended/jwt_manager.py#L96

This error will only show up if you try to pass in data that does not resemble a JWT at all, for example a string that does not have three dots and three different sections per the JWT spec. If you are getting a 500 error on an expired token you have something else going wrong.

If you are using something like Flask-Restful, you need to set app.config['PROPAGATE_EXCEPTIONS'] = True for the error to be properly returned to the caller. Or if you are using Flask-Restplus there is an bug with their error handling, which can be worked around like such: #86 (comment)

@mybooc
Copy link

mybooc commented Apr 23, 2021

Thank you for your great work. Also bounced into something like this caused by a bad header, e.g. 'Bearer ,'. This causes a 500 error:

File "/home/nginxwww/xena/httpdocs/flask_test/venvs/rest_swag2m/lib/python3.8/site-packages/flask_jwt_extended/view_decorators.py", line 152, in <listcomp> jwt_header = [s for s in field_values if s.split()[0] == header_type] IndexError: list index out of range

This is caused by the line in view_decorators.py:

jwt_header = [s for s in field_values if s.split()[0] == header_type]

My suggestion is to handle this by changing the code and raising a NoAuthorizationError:

`

try:
   jwt_header = [s for s in field_values if s.split()[0] == header_type]
except Exception as e:
    raise NoAuthorizationError("Failed to decode Header: {}".format(auth_header))

`
I do not see any harm in doing this.

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

No branches or pull requests

3 participants