Match inline event handlers in JavaScript files #18
Conversation
|
@daniel-beck I've also noticed in Java patterns we don't account for inline handlers being inside single quotes or no quotes at all. Was that intentional by chance? |
| @@ -123,7 +125,7 @@ private static void visitFile(File file, Set<Match> matches) throws IOException | |||
|
|
|||
| if (fileName.endsWith(".java")) { | |||
| final String text = readFileToString(file); | |||
| printMatches(matchRegexes(JAVA_PATTERNS, text, file)); | |||
| matches.addAll(matchRegexes(JAVA_PATTERNS, text, file)); | |||
There was a problem hiding this comment.
Unsure if it was done intentionally. Will revert if was.
Could you provide a code example where that would happen? |
I can imagine that being return " onclick='fetch(decodeURIComponent(atob('" + encodeForJavascript(url) + "')), { method: 'post', headers: crumb.wrap({})}); return false'";or return " onclick=fetch(decodeURIComponent(atob('" + encodeForJavascript(url) + "')), { method: 'post', headers: crumb.wrap({})}); return false"; |
|
I don't see the problem. Adding those two suggested lines to the original, I get: Are you misinterpreting the quote matching that accounts for all hits in a Java source file necessarily being part of a double-quoted Java string as being part of the HTML attribute match? |
Hm, I've also checked and all of the cases are matched. Then I should revisit the regex I'm adding, likely can be simplified. |
| @@ -36,7 +36,9 @@ public class Scanner { | |||
| // Examples indicate trying to match open-paren would be too restrictive: | |||
| // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/eval#direct_and_indirect_eval | |||
| // geval is defined in hudson-behavior.js | |||
| protected static final Map<String, Pattern> JS_PATTERNS = Map.of("(g)eval Call", Pattern.compile("\\Wg?eval\\W")); | |||
| protected static final Map<String, Pattern> JS_PATTERNS = Map.of("(g)eval Call", Pattern.compile("\\Wg?eval\\W"), | |||
| "Inline Event Handler (JavaScript)", Pattern.compile("[\\s+]" + JS_EVENT_ATTRIBUTES + ".*?((?<!\\\\)['\"]?)[_\\w]+\\s*\\(", Pattern.CASE_INSENSITIVE), | |||
There was a problem hiding this comment.
| "Inline Event Handler (JavaScript)", Pattern.compile("[\\s+]" + JS_EVENT_ATTRIBUTES + ".*?((?<!\\\\)['\"]?)[_\\w]+\\s*\\(", Pattern.CASE_INSENSITIVE), | |
| "Inline Event Handler (JavaScript)", Pattern.compile("[\\s+]" + JS_EVENT_ATTRIBUTES + "=.*(['"]?)[$_\\w]+\\s*\\(", Pattern.CASE_INSENSITIVE), |
Could simplify to this. [$_\\w]+ is not a complete regex for valid JS identifiers though. But maybe good enough?
Can also be
| "Inline Event Handler (JavaScript)", Pattern.compile("[\\s+]" + JS_EVENT_ATTRIBUTES + ".*?((?<!\\\\)['\"]?)[_\\w]+\\s*\\(", Pattern.CASE_INSENSITIVE), | |
| "Inline Event Handler (JavaScript)", Pattern.compile("[\\s+]" + JS_EVENT_ATTRIBUTES + "=.*(['"]?)", Pattern.CASE_INSENSITIVE), |
This will match the rest of the line after the handler.
Addresses #16.
Known limitation:
If JavaScript is poorly formatted may result in false positives. E.g.
document. onload=function(){}will get matched.