Fixes CSRF when working along side sessions #1613
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Logs should have a lower level otherwise they will pollute the user logs with debug information |
|
Hi, Thank you for looking into it in such a short time. I am happy that my vulnerability report contributed into making this framework more secure. Keep up the good work! Best, Xhelal Likaj |
803e9b1 to
f5b3464
Compare
…e sessions Signed-off-by: Paulo Lopes <pmlopes@gmail.com>
Signed-off-by: Paulo Lopes <pmlopes@gmail.com>
f5b3464 to
41a5534
Compare
|
CVE-2020-35217 was assigned to this PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Paulo Lopes pmlopes@gmail.com
Motivation:
It was brought up to us by @xhlika that the current implementation of
CSRFon the development branch of what will become4.0.0suffered from several security issues when working with session aware tokens. The bug was introduced with: #1465 and totally ignored the validation of the client side input but rather check what was on the session.This PR properly fixes it by following the updated recommendations of OWASP https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#double-submit-cookie which have been updated Feb 2020 and include new checks that were not performed then and the notes provided by @xhlika on the potential attack vector.
Tests have been fixed to cover replay attacks on session mode and proper token expiration is tested.
The PR also addresses adding the
SameSitepolicy to the cookie (which is a recently added feature to vert.x core and wasn't available at the time of #1465.Further, it also cover validating the request origin as per the OWASP recommendations.