[WIP] ascanrules: Fix for XSS issues related to javascript context.

[jokrsec]

Fixes zaproxy/zaproxy#7212

Integrated a javascript parser to extract the contexts. Updated the scanner to fix the issues.

[kingthorin]

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.)

[jokrsec]

The parser code is generated by antlr. I'll add the unit tests for the fixed issues 🙂

[kingthorin]

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.

[psiinon]

It does say // Generated from JavaScriptLexer.g4 by ANTLR 4.10.1 :)

[kingthorin]

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.)

[psiinon]

FYI I've been chatting to @jokrhub about this, and will manually override the DCO check :)

[kingthorin]

If the antlr files are generated we should also document the steps needed to update.

[kingthorin]

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.

[kingthorin]

Looks good overall. The DCO and squash/fixup still need to be addressed. Plus there's one thing here that could probably be simplified.

[kingthorin]

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.

[jokrsec]

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.

[kingthorin]

Setting DCO pass and will on subsequent commits as well.

[thc202]

We should add the grammar and a Gradle task to (re)generate the code.

[kingthorin]

@jokrhub if you can just add a comment with some details on how the code was generated we can discuss adding a gradle task 😀

[jokrsec]

Below are the steps to generate lexer and parser files using antlr:

1. 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
   

2. Generate java files for lexer and parser

    java -jar antlr-4.10.1-complete.jar JavaScriptLexer.g4 JavaScriptParser.g4
   

3. Choose java files among the generated files.

[kingthorin]

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.

[kingthorin]

Found it: 7beed33

[kingthorin]

Restored.

[thc202]

Thanks! Feel free to fix up if it looks sane.

[kingthorin]

It does look sane. I still need to address earlier comments about logic changes and additional tests.

Which I intend to tackle but I need to take the time to figure out what the original logic was catching, write tests, then figure out why/how the logic was changed. (Unless @jokrhub or someone else has interest in tackling that.)

All that to say, there’s still more change coming that will probably also need to be flattened.

[thc202]

Sounds good, I can take a look at adding the tests cases.

[kingthorin]

Thanks, I'd appreciate that!

[kingthorin]

Rebased current

[github-actions]

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.

---

I have read the CLA Document and I hereby sign the CLA

---

1 out of 2 committers have signed the CLA.
✅ @thc202
❌ @jokrsec

[kingthorin]

This is pending me fixing things after the rebase. Between the previous one and now another 10 param method was introduced so now that conflicts with these changes.