zap-extensions icon indicating copy to clipboard operation
zap-extensions copied to clipboard

pscanrules: Address Suspicious Comments rule JS FPs

Open kingthorin opened this issue 1 year ago • 2 comments

Overview

  • CHANGELOG > Added fix note.
  • InformationDisclosureSuspiciousCommentsScanRule > Updated handling to target comments in JavaScript more specifically.
  • InformationDisclosureSuspiciousCommentsScanRuleUnitTest b> Updated and added tests.
  • Messages.properties > Updated to detail/report the findings more specifically based on the new behavior.
  • pscanrules.html > Correct occurrence of "add-on" (vs addon).

Note: The regexes used for JS comments are based on https://github.com/antlr/grammars-v4/blob/c82c128d980f4ce46fb3536f87b06b45b9619922/javascript/javascript/JavaScriptLexer.g4#L49-L50

Related Issues

  • Fixes zaproxy/zaproxy#6622
  • Fixes zaproxy/zaproxy#6736

Checklist

  • [x] Update help
  • [x] Update changelog
  • [x] Run ./gradlew spotlessApply for code formatting
  • [x] Write tests
  • [x] Check code coverage
  • [x] Sign-off commits
  • [x] Squash commits
  • [x] Use a descriptive title

kingthorin avatar Oct 13 '24 02:10 kingthorin

Tweaked

kingthorin avatar Oct 13 '24 11:10 kingthorin

Fixed

kingthorin avatar Oct 17 '24 10:10 kingthorin

There's a typo in the commit/PR description b>.

thc202 avatar Oct 25 '24 07:10 thc202

As far as performance goes here's what I've been able to find:

Live perf examples
https://www.bell.ca/Bell-bundles/Internet-Mobility

Old

78160783 [ZAP-IO-Server-1-44] INFO  org.zaproxy.zap.extension.websocket.WebSocketProxy - collection.decibelinsight.net:443 (#6) is excluded from storage & UI!
78161331 [ZAP-PassiveScan-2] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 7 seconds to scan https://www.bell.ca/Bell-bundles/Internet-Mobility text/html; charset=UTF-8 961304
78165329 [ZAP-PassiveScan-5] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 11 seconds to scan https://assets.adobedtm.com/launch-ENebd7a9b148404f67903d514c40949f24.min.js application/x-javascript 567091
78165338 [ZAP-PassiveScan-6] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 7 seconds to scan https://www.bell.ca/ruxitagentjs_ICA7NQVfghqrux_10299241001084140.js text/javascript; charset=utf-8 341434
78166382 [ZAP-PassiveScan-8] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 5 seconds to scan https://www.bell.ca/ruxitagentjs_D_10299241001084140.js text/javascript; charset=utf-8 43211
78168938 [ZAP-PassiveScan-4] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 5 seconds to scan https://www.googletagmanager.com/gtag/js?id=AW-953414520 application/javascript; charset=UTF-8 289075
78170511 [ZAP-PassiveScan-1] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 6 seconds to scan https://www.googletagmanager.com/gtag/js?id=G-MTKGWZ28E4 application/javascript; charset=UTF-8 344100
78173582 [ZAP-PassiveScan-8] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 5 seconds to scan https://connect.facebook.net/en_US/fbevents.js application/x-javascript; charset=utf-8 234260
78179114 [ZAP-PassiveScan-4] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 5 seconds to scan https://googleads.g.doubleclick.net/pagead/viewthroughconversion/953414520/?random=1730373038776&cv=11&fst=1730373038776&bg=ffffff&guid=ON&async=1&gtm=45be4as0v9172277568za200&gcd=13l3l3l3l1l1&dma=0&tag_exp=101533421~101823848~101878899~101878944~101925629&u_w=1920&u_h=1080&url=https%3A%2F%2Fwww.bell.ca%2FBell-bundles%2FInternet-Mobility&hn=www.googleadservices.com&frm=0&tiba=Bundles%20%7C%20Internet%20%26%20Mobility%20%7C%20Bell%20Canada&npa=0&pscdl=noapi&auid=1298872184.1730373035&fdr=QA&data=event%3Dgtag.config&rfmt=3&fmt=4 text/javascript; charset=UTF-8 5354
78179256 [ZAP-PassiveScan-8] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 5 seconds to scan https://www.googletagmanager.com/static/service_worker/4al0/sw.js?origin=https%3A%2F%2Fwww.bell.ca text/javascript 7076
78180880 [ZAP-PassiveScan-1] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 10 seconds to scan https://www.bell.ca/Styles/media/Shared/js/HowtoBuy.js?v=1.0.9 application/javascript 192214
78180911 [ZAP-PassiveScan-5] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 9 seconds to scan https://www.gstatic.com/recaptcha/releases/lqsTZ5beIbCkK4uGEGv9JmUR/recaptcha__en.js text/javascript 557225
78180911 [ZAP-PassiveScan-6] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 9 seconds to scan https://www.gstatic.com/recaptcha/releases/lqsTZ5beIbCkK4uGEGv9JmUR/recaptcha__en.js text/javascript 557225
78180933 [ZAP-PassiveScan-3] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 8 seconds to scan https://www.gstatic.com/recaptcha/releases/lqsTZ5beIbCkK4uGEGv9JmUR/recaptcha__en.js text/javascript 557225

New
78022884 [ZAP-PassiveScan-7] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 9 seconds to scan https://www.bell.ca/Bell-bundles/Internet-Mobility text/html; charset=UTF-8 961318
78034777 [ZAP-PassiveScan-4] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 10 seconds to scan https://www.bell.ca/eShop/Qualification/HowtoBuyInternet?gigabitPreFlow=&shopQualModal=true&resPath=%2FBRSeShop%2Fqualification%2Fcentral_shop_internet&returnUrl=https%3A%2F%2Fwww.bell.ca%2FBell-bundles%2FInternet-Mobility&framework=brf text/html; charset=UTF-8 294957
78058621 [ZAP-IO-Server-1-32] INFO  org.zaproxy.zap.extension.websocket.WebSocketProxy - collection.decibelinsight.net:443 (#5) is excluded from storage & UI!
78059381 [ZAP-PassiveScan-5] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 7 seconds to scan https://www.bell.ca/Bell-bundles/Internet-Mobility text/html; charset=UTF-8 961302
78070658 [ZAP-PassiveScan-7] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 6 seconds to scan https://www.bell.ca/Styles/BRF/content/js/eshop/QualificationCommon.js application/javascript 14045
78071073 [ZAP-PassiveScan-3] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 5 seconds to scan https://www.clarity.ms/s/0.7.49/clarity.js application/javascript;charset=utf-8 65959
78072525 [ZAP-PassiveScan-4] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 5 seconds to scan https://googleads.g.doubleclick.net/pagead/viewthroughconversion/953414520/?random=1730372936537&cv=11&fst=1730372936537&bg=ffffff&guid=ON&async=1&gtm=45be4as0v9172277568za200&gcd=13l3l3l3l1l1&dma=0&tag_exp=101533422~101823848~101878899~101878944~101925629&u_w=1920&u_h=1080&url=https%3A%2F%2Fwww.bell.ca%2FBell-bundles%2FInternet-Mobility&hn=www.googleadservices.com&frm=0&tiba=Bundles%20%7C%20Internet%20%26%20Mobility%20%7C%20Bell%20Canada&npa=0&pscdl=noapi&auid=520820797.1730372932&fdr=QA&data=event%3Dgtag.config&rfmt=3&fmt=4 text/javascript; charset=UTF-8 5354
78073935 [ZAP-PassiveScan-5] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 11 seconds to scan https://www.bell.ca/eShop/Qualification/HowtoBuyInternet?gigabitPreFlow=&shopQualModal=true&resPath=%2FBRSeShop%2Fqualification%2Fcentral_shop_internet&returnUrl=https%3A%2F%2Fwww.bell.ca%2FBell-bundles%2FInternet-Mobility&framework=brf text/html; charset=UTF-8 294957
78074004 [ZAP-PassiveScan-2] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 9 seconds to scan https://www.bell.ca/eShop/Qualification/HowtoBuyInternet?gigabitPreFlow=&shopQualModal=true&resPath=%2FBRSeShop%2Fqualification%2Fcentral_bundle&returnURL=https%3A%2F%2Fwww.bell.ca%2FBell-bundles%2FInternet-Mobility&framework=brf text/html; charset=UTF-8 280536

New after pre-check

78607448 [ZAP-IO-Server-1-56] INFO  org.zaproxy.zap.extension.websocket.WebSocketProxy - collection.decibelinsight.net:443 (#7) is excluded from storage & UI!
78608202 [ZAP-PassiveScan-6] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 7 seconds to scan https://www.bell.ca/Bell-bundles/Internet-Mobility text/html; charset=UTF-8 961304
78621906 [ZAP-PassiveScan-4] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 9 seconds to scan https://www.bell.ca/eShop/Qualification/HowtoBuyInternet?gigabitPreFlow=&shopQualModal=true&resPath=%2FBRSeShop%2Fqualification%2Fcentral_shop_internet&returnUrl=https%3A%2F%2Fwww.bell.ca%2FBell-bundles%2FInternet-Mobility&framework=brf text/html; charset=UTF-8 294957

I'm not sure how to add a test based on that? 🤷

kingthorin avatar Oct 31 '24 11:10 kingthorin

How are you running the tests?

thc202 avatar Oct 31 '24 11:10 thc202

That was just browsing with only the one rule enabled. Opening a new browser form ZAP after re-installing different versions of the add-on.

kingthorin avatar Oct 31 '24 12:10 kingthorin

Deconflicted

kingthorin avatar Nov 17 '24 12:11 kingthorin

Now also skips font requests:

169870806 [ZAP-PassiveScan-5] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 5 seconds to scan https://www.bell.ca/styles/media/Mobility/css/fonts/shop-icons.woff2 text/html; charset=utf-8 190704
169871023 [ZAP-PassiveScan-4] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 5 seconds to scan https://www.bell.ca/styles/media/Mobility/css/fonts/bellslim_black-webfont.woff2 text/html; charset=utf-8 190704
169871096 [ZAP-PassiveScan-7] WARN  org.zaproxy.zap.extension.pscan.PassiveScanTask - Passive Scan rule Information Disclosure - Suspicious Comments took 5 seconds to scan https://www.bell.ca/styles/media/Mobility/css/fonts/shop-icons.woff text/html; charset=utf-8 190702

kingthorin avatar Nov 17 '24 13:11 kingthorin

I've rebased this and added a test that covers the default payloads in JS single line comments.

kingthorin avatar Feb 20 '25 15:02 kingthorin

Logo Checkmarx One – Scan Summary & Details1ba12235-291e-4841-8109-fd7d2ba81af7

Great job, no security vulnerabilities found in this Pull Request

psiinon avatar Feb 20 '25 15:02 psiinon

Tweaked

kingthorin avatar Feb 21 '25 11:02 kingthorin

Thank you!

thc202 avatar Feb 21 '25 11:02 thc202