zap-extensions
zap-extensions copied to clipboard
zaproxy
[WIP] ascanrules: Fix for XSS issues related to javascript context.
Fixes https://github.com/zaproxy/zaproxy/issues/7212
Integrated a javascript parser to extract the contexts. Updated the scanner to fix the issues.
Is some of this generated or third party code? Does this increase coverage? (If so unit tests should be added to demonstrate the new coverage and ensure it isn't broken in the future.)
The parser code is generated by antlr. I'll add the unit tests for the fixed issues 🙂
The parser code is generated by antlr.
This should be noted somehow, so that people don't try to maintain the classes manually. (@thc202 advice/ideas on that front?)
I'll add the unit tests for the fixed issues
Great.
There's unfortunately also a conflict you're going to need to address. I think I mentioned on the other PR that I'd look at it but I haven't had a chance yet. The XSS scan rule was made less monolithic so hopefully adapting your changes isn't too bad/hard.
That's a good start, sorry I overlooked it. Should we exclude the license header from those then? (I think that will help it to stand out.)
FYI I've been chatting to @jokrhub about this, and will manually override the DCO check :)
If the antlr files are generated we should also document the steps needed to update.
This pull request introduces 1 alert when merging 1610c8f3d35a81dddc8df31a7231ef99ffccd5a4 into f22b9a1ede532f13b476bfd195f286f06ec2c907 - view on LGTM.com
new alerts:
- 1 for Boxed variable is never null
![lgtm-com[bot] avatar](https://avatars.githubusercontent.com/in/17324?v=4 "Published by a pub.dev verified publisher"){kind=small}
This pull request introduces 1 alert when merging 1610c8f3d35a81dddc8df31a7231ef99ffccd5a4 into f22b9a1ede532f13b476bfd195f286f06ec2c907 - view on LGTM.com
new alerts:
- 1 for Boxed variable is never null
To address the DCO requirement you'll need to sign-off the commit(s):
- https://github.com/zaproxy/zaproxy/blob/main/CONTRIBUTING.md#developer-certificate-of-origin
- https://git-scm.com/docs/git-commit#Documentation/git-commit.txt---signoff
You could also fixup all the commits into a single and force push. Then use commit --amend and force push on subsequent changes
- https://www.kernel.org/pub/software/scm/git/docs/git-rebase.html#_interactive_mode
- https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History
- https://www.instructables.com/id/How-to-Rebase-Interactive-With-Eclipse-eGit/
- https://wiki.eclipse.org/EGit/User_Guide#Interactive_Rebase.
We spoke on slack, I understand you're having trouble getting an interactive rebase to work nicely. Don't worry, it isn't terribly straight forward and is daunting if you haven't done it successfully in the past.
If you can just confirm that you agree with https://developercertificate.org/ we can manually set it to pass.
We spoke on slack, I understand you're having trouble getting an interactive rebase to work nicely. Don't worry, it isn't terribly straight forward and is daunting if you haven't done it successfully in the past.
If you can just confirm that you agree with https://developercertificate.org/ we can manually set it to pass.
Yes I agree.
Setting DCO pass and will on subsequent commits as well.
@jokrhub if you can just add a comment with some details on how the code was generated we can discuss adding a gradle task :grinning:
Below are the steps to generate lexer and parser files using antlr:
-
Download antlr jar file, javascript grammar files and base files for lexer and parser. Place them in the same folder.
wget https://www.antlr.org/download/antlr-4.10.1-complete.jar wget https://raw.githubusercontent.com/antlr/grammars-v4/master/javascript/javascript/JavaScriptLexer.g4 wget https://raw.githubusercontent.com/antlr/grammars-v4/master/javascript/javascript/JavaScriptParser.g4 wget https://raw.githubusercontent.com/antlr/grammars-v4/master/javascript/javascript/Java/JavaScriptLexerBase.java wget https://raw.githubusercontent.com/antlr/grammars-v4/master/javascript/javascript/Java/JavaScriptParserBase.java -
Generate java files for lexer and parser
java -jar antlr-4.10.1-complete.jar JavaScriptLexer.g4 JavaScriptParser.g4 -
Choose java files among the generated files.
The ANTLR tool version 4.10.x requires Java 11, the code will be generated and executed with version 4.9.x which still supports Java 8.
The ANTLR tool version 4.10.x requires Java 11, the code will be generated and executed with version 4.9.x which still supports Java 8.
Have we simplified this now that we’re targeting 11?
I started to re-implement this, in order to flatten it and address the conflicts. I got as far as UnitTests when I discovered it doesn't want to build on windows:
Console output of the issue I encountered
$ ./gradlew :aO:ascanrules:assemble
> Task :buildSrc:jar
:jar: No valid plugin descriptors were found in META-INF/gradle-plugins
> Configure project :addOns:browserView
Project :addOns:browserView => no module-info.java found
> Task :addOns:ascanrules:compileJava FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':addOns:ascanrules:compileJava'.
> Illegal/unsupported escape sequence near index 3
C:\Users\thorin\Desktop\zap-ws\zap-extensions\addOns\ascanrules\build\generated-src\antlr\main.*
^
* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.
* Get more help at https://help.gradle.org
BUILD FAILED in 9s
22 actionable tasks: 5 executed, 3 from cache, 14 up-to-date
I tried adjusting ascanrules.gradle.kts to build the antlr paths with \\ instead of / and it still failed.
Grr I don’t see the push here anymore, got the id? Or can you just push it again and I’ll deconflict things more carefully.
Found it: https://github.com/zaproxy/zap-extensions/commit/7beed333bad3dba30e46600224c45d18c34fae41