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

Remove use of innerHtml #621

Open
1 of 3 tasks
psiinon opened this issue Oct 8, 2019 · 6 comments
Open
1 of 3 tasks

Remove use of innerHtml #621

psiinon opened this issue Oct 8, 2019 · 6 comments
Assignees
Labels
maintenance Code or build changes that make the project easier to maintain

Comments

@psiinon
Copy link
Member

psiinon commented Oct 8, 2019

Re #620

  • zap-hud/src/main/zapHomeFiles/hud/display.js
    • Tricky - we are injecting the HTML ZAP returns - this will require significant rework
  • zap-hud/src/main/zapHomeFiles/hud/libraries/alertify.js
  • zap-hud/src/main/zapHomeFiles/hud/target/inject.js
    • This should be an easy fix
@psiinon psiinon added the maintenance Code or build changes that make the project easier to maintain label Oct 8, 2019
@mozfreddyb
Copy link
Contributor

Re #620
* [ ] zap-hud/src/main/zapHomeFiles/hud/display.js
* Tricky - we are injecting the HTML ZAP returns - this will require significant rework

If you're not against introducing a sanitizer (like DOMPurify), you could change this from
window.open('').document.body.innerHTML = event.data.response;
to
window.open('').document.body.innerHTML = purify(event.data.response);

and have the linter explicitly allow innerHTML as long as it's used in combination with dompurify.

@psiinon
Copy link
Member Author

psiinon commented Oct 9, 2019

I have no problem adding DOMPurify to the ZAP domain - it could definitely prove useful. We're trying to limit what we include on the target domain, but that doesnt apply in this case.

@psiinon psiinon self-assigned this Oct 9, 2019
@mozfreddyb
Copy link
Contributor

if I were to add dompurify as a dependency in package.json - do you have a build step to move a copy to src/main/zapHomeFiles/hud/libraries?

@mozfreddyb
Copy link
Contributor

Ah, I did not see you self-assigned. Nevermind then :-) I'll modify #620 to have the linter accept dompurify as an exception

@psiinon
Copy link
Member Author

psiinon commented Oct 9, 2019

Yes, the HUD runtime JS is delivered by ZAP so we need it there.
And please got ahead with that change - I was going to look at the 3rd one first :)

@thc202
Copy link
Member

thc202 commented Oct 9, 2019

do you have a build step to move a copy to src/main/zapHomeFiles/hud/libraries?

Which ones are copied are tracked here:

zap-hud/build.gradle.kts

Lines 96 to 106 in c1839b1

val copyNpmDeps by tasks.registering(Copy::class) {
group = LifecycleBasePlugin.BUILD_GROUP
description = "Copies the (required) npm dependencies for the add-on."
dependsOn(installNpmDeps)
from("node_modules/vue/dist/vue.js")
from("node_modules/vue-i18n/dist/vue-i18n.js")
from("node_modules/localforage/dist/localforage.min.js")
into(npmDepsDir.map({ it.file("hud/libraries/") }))
}

psiinon added a commit to psiinon/zap-hud that referenced this issue Oct 10, 2019
Part of fix for zaproxy#621

Signed-off-by: Simon Bennetts <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Code or build changes that make the project easier to maintain
Development

No branches or pull requests

3 participants