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

Ignore Javascript Files while scanning for timestamps #6037

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

njmulsqb
Copy link
Contributor

Passive scan rule identifies timestamps falsely in js files ending up in false positives. Timestamps in JS files can't happen so they should be ignored.

fixes zaproxy/zaproxy#8380

Signed-off-by: Najam Ul Saqib <najamulsaqib@tutamail.com>
@psiinon
Copy link
Member

psiinon commented Dec 24, 2024

Logo
Checkmarx One – Scan Summary & Details9282ca23-b003-4c65-b7eb-a113e0ad14c8

No New Or Fixed Issues Found

@kingthorin
Copy link
Member

Timestamps can absolutely happen JS files, there are frameworks that generate JS based on user profiles etc with user specifications fix constants etc

Maybe excluding JS should be tied to threshold, if this is the proposed fix.

@kingthorin
Copy link
Member

Should include a changelog entry, updated/new tests, and if tied to threshold a help update.

@njmulsqb
Copy link
Contributor Author

Interesting, so by threshold you mean we should ignore it if there are more than N amount of findings in .js files? Given the fact you mentioned, if there can be timestamps in js files than all we need is a one valid finding among 499 others, right?

@kingthorin
Copy link
Member

No I mean use the getAlertThreshold method.

https://www.zaproxy.org/docs/desktop/ui/dialogs/options/pscanrules/#threshold

At High it could skip JS to raise fewer potential issues.

If you search the passive scan rules you'll find other examples.

@njmulsqb
Copy link
Contributor Author

That's sounds like a better implementation.

Let's wait for others (@psiinon @thc202 ) to agree on this solution before I work in that direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

False Positive - Timestamp Disclosure
3 participants