-
Notifications
You must be signed in to change notification settings - Fork 217
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: Verify a given 'mimeType' and/or 'callback' request parameter
So that only fixed values are possible, in order to avoid XSS attack vectors.
- Loading branch information
Showing
5 changed files
with
242 additions
and
28 deletions.
There are no files selected for viewing
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
74 changes: 74 additions & 0 deletions
74
agent/core/src/main/java/org/jolokia/util/MimeTypeUtil.java
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| package org.jolokia.util; | ||
|
|
||
| import java.util.regex.Pattern; | ||
|
|
||
| /** | ||
| * Helper class for handling proper response mime types | ||
| * | ||
| * @author roland | ||
| * @since 24.01.18 | ||
| */ | ||
| public class MimeTypeUtil { | ||
|
|
||
|
|
||
| /** | ||
| * Extract the response mime type. This value is calculated for different situations: | ||
| * <p> | ||
| * <ul> | ||
| * <li>If a callback is given and its valid, the mime type is "text/javascript"</li> | ||
| * <li>Otherwise: | ||
| * <ul> | ||
| * <li>If a valid mimeType is given in the request ("text/plain", "application/json"), then this | ||
| * mimet type is returned</li> | ||
| * <li>If another mimeType is given, then "text/plain" is used</li> | ||
| * <li>If no mimeType is given then a given default mime type is used, but also sanitized | ||
| * as described above</li> | ||
| * </ul> | ||
| * </li> | ||
| * </ul> | ||
| * | ||
| * @param pRequestMimeType the mimetype given in the request | ||
| * @param defaultMimeType the default mime type to use if none is given in the request | ||
| * @param pCallback a callback given (can be null) | ||
| */ | ||
| public static String getResponseMimeType(String pRequestMimeType, String defaultMimeType, String pCallback) { | ||
|
|
||
| // For a valid given callback, return "text/javascript" for proper inclusion | ||
| if (pCallback != null && isValidCallback(pCallback)) { | ||
| return "text/javascript"; | ||
| } | ||
|
|
||
| // Pick up mime time from request, but sanitize | ||
| if (pRequestMimeType != null) { | ||
| return sanitize(pRequestMimeType); | ||
| } | ||
|
|
||
| // Use the given default mime type (possibly picked up from a configuration) | ||
| return sanitize(defaultMimeType); | ||
| } | ||
|
|
||
| private static String sanitize(String mimeType) { | ||
| for (String accepted : new String[]{ | ||
| "application/json", | ||
| "text/plain" | ||
| }) { | ||
| if (accepted.equalsIgnoreCase(mimeType)) { | ||
| return accepted; | ||
| } | ||
| } | ||
| return "text/plain"; | ||
| } | ||
|
|
||
| /** | ||
| * Check that a callback matches a javascript function name. The argument must be not null | ||
| * | ||
| * @param pCallback callback to verify | ||
| * @return true if valud, false otherwise | ||
| */ | ||
| public static boolean isValidCallback(String pCallback) { | ||
| Pattern validJavaScriptFunctionNamePattern = | ||
| Pattern.compile("^[$A-Z_][0-9A-Z_$]*$", Pattern.CASE_INSENSITIVE); | ||
| return validJavaScriptFunctionNamePattern.matcher(pCallback).matches(); | ||
| } | ||
|
|
||
| } |
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
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
Oops, something went wrong.
5895d5cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how can test the xss attack?
5895d5cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The XSS attack happens only when you are using the Jolokia response directly in an browser and when you set the mimeType to e.g. text/html and provoke an error with an error message containing html code. But as you normally consume Jolokia responses via some client lib (javascript or java) you are not directly affected normally. I.e. I dont know any app showing Jolokia JSON responses directly in the browser.
5895d5cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello,
I think that the CVE is not correctly declared so SCA/dependency analyser (like dependency-check) are not able to detect it.
See https://nvd.nist.gov/vuln/detail/CVE-2018-1000129
Configuration 1 OR cpe:2.3:a:jolokia:jolokia:1.3.7:*:*:*:*:*:*:*Only version 1.3.7 will be detected. I think it should be something like:
cpe:2.3:a:jolokia:jolokia:*:*:*:*:*:*:*:* versions up to (excluding) 1.5.0Thanks,
5895d5cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adioss actually it was not me who opened the CVE but Martin Hopkins from GDS. I suggest that you contact him for the update ?
5895d5cThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rhuss I will do that. Thanks a lot for the great job that you do for this project.
Adrien