rspec/rules/S5659/python/comments-and-links.adoc

54 lines
2.6 KiB
Plaintext

=== on 22 Feb 2021, 11:17:11 Pierre-Yves Nicolas wrote:
The first code example of the current description of the RSPEC links to https://github.com/GehirnInc/python-jwt but this is another JWT implementation.
There are multiple JWT implementations in Python with very similar names and they sometimes use the same namespace...
* PyJWT: https://github.com/jpadilla/pyjwt
https://pyjwt.readthedocs.io/en/latest/index.html[doc]
This is the one which matches the first code example of the current description.
https://pyjwt.readthedocs.io/en/latest/changelog.html#v2-0-0[Starting with version 2.0], the verify param of the decode function was dropped and replaced with
----
jwt.decode(encoded, key, options={"verify_signature": False}){code}
* [https://github.com/davedoesdev/python-jwt]
This is the one which has a process_jwt function.
{quote}From version 2.0.1 the namespace has changed from jwt to python_jwt, in order to avoid conflict with PyJWT.
{quote}
* [https://github.com/GehirnInc/python-jwt]
This implementation is wrongly linked in the current description of the ticket.
It's actually not covered by the current description of the RSPEC.
----
=== on 22 Feb 2021, 14:06:26 Pierre-Yves Nicolas wrote:
\[~hendrik.buchwald] Should we raise an issue on the following code?
----
try:
jwt.decode(token, key, algo)
except:
raise Exception("Invalid token")
----
Real world cases:
* \https://github.com/Bounties-Network/BountiesAPI/blob/7931957e56c9fcef574dd3a00ec5ab9a787a365c/bounties_api/user/middleware.py#L20
* \https://github.com/DragonComputer/Dragonfire/blob/dd21f8e88d9b6390bd229ff73f89a8c3c137b89c/dragonfire/api.py#L47
* \https://github.com/JeffVandrewJr/patron/blob/b2545066c8e57e398f4896eb9b25fb4e970e0f06/app/models.py#L125
=== on 22 Feb 2021, 17:31:35 Pierre-Yves Nicolas wrote:
\[~hendrik.buchwald] Shouldn't we drop the last part of the rule title ("with strong cipher algorithms") since we don't check the algorithm?
=== on 24 Feb 2021, 10:42:09 Hendrik Buchwald wrote:
\[~pierre-yves.nicolas] thanks, good catch! Luckily I only confused the links, the first one was supposed to be PyJWT (as that one is used much more often). I will change the name of the rule.
That are nice examples for the exceptions. While they are of course secure I think it might be fine to still raise this issue since there is no reason to not check for the right type of exception (e.g. ``++jwt.InvalidSignatureError++``). The intention of this specification is though to detect cases where the invalid signature exception is caught accidentally.
=== on 24 Feb 2021, 13:09:31 Hendrik Buchwald wrote:
I have removed the exception case for now.
include::../comments-and-links.adoc[]