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

feat: implement header param injection handling for JWT vulnerabilities #473

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

leiberbertel
Copy link

Added handling for header parameter injection in JWTVulnerability.java, addressing the missing attack vector noted in issue #413. Follows https://portswigger.net/web-security/jwt guidelines.

Ref: #413

@preetkaran20 preetkaran20 self-requested a review August 19, 2024 17:19
public ResponseEntity<GenericVulnerabilityResponseBean<String>> getHeaderInjectionVulnerability(
HttpServletRequest request) {
String headerValue = request.getHeader("User-Defined-Header");
if (headerValue != null && headerValue.contains("malicious")) {
Copy link
Member

@preetkaran20 preetkaran20 Aug 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@leiberbertel I think this is not the right way to implement the header param injection. The real vulnerability is with optional headers like JWK or KID or JKU as implemention of library will start relying on the data provided as part of these headers for verifying signature.

Having custom/user defined headers is not wrong or considered malicious.

A simple attack is say JWT library is relying on the public key provided as part of JWK header instead of validating the public key provided with the whitelisted public key set. so attack would be a malicious user will generate key pairs and sign the jwt with its own private key and passed public key as JWK header. The backend will not validate public key against a whitelist and start relying on provided public key to validate the JWT.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @preetkaran20,
I really appreciate your detailed review and the points you highlighted. I completely agree that attacks via headers such as JWK, KID or JKU pose a significant threat, especially when the implementation does not properly validate public keys against a trusted set.

My intent with the current implementation was to demonstrate how user-defined headers can be manipulated, but I see that focusing on these critical headers such as JWK could provide a more realistic and educational example of common vulnerabilities in JWTs.

I will revise the implementation to incorporate these examples, ensuring that we clearly demonstrate how the lack of proper validation of public keys can be exploited.

Regards.

@@ -245,6 +245,11 @@ function doGetAjaxCall(callBack, url, isJson) {
"Content-Type",
isJson ? "application/json" : "text/html"
);

for (const header in headers) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this change.

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 this pull request may close these issues.

2 participants