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

Provide example alerts in Cookie scan rules #2532

Merged

Conversation

martinspielmann
Copy link
Contributor

@martinspielmann martinspielmann commented Sep 3, 2020

The CWE id was already managed for these Alerts/ScanRules and within ZAP they were already presented to the user.
However, when visiting the alert detail pages online (https://www.zaproxy.org/docs/alerts/), CWE (and WASC) links were not generated correctly.

The reason is, that the id values were set inline to the alert.
For the script which generates the detail pages however, the getter methods for the ids have to be added publicly to the class.

Note: if this solves the issue, I'll vote for another follow up issue to add this change to the PluginPassiveScanner parent class,
so that the docs will be generated consistently over all Passive Scan Rules.

Fixes zaproxy/zaproxy#6140.

@thc202 thc202 changed the title Expose getCweId and getWascId method for Cookie scan rules. Fixes #6140 Expose getCweId and getWascId method for Cookie scan rules Sep 3, 2020
@thc202
Copy link
Member

thc202 commented Sep 3, 2020

Note: if this solves the issue, I'll vote for another follow up issue to add this change to the PluginPassiveScanner parent class, so that the docs will be generated consistently over all Passive Scan Rules.

It does, but there's a new way to provide the alert data, more details in zaproxy/zaproxy#6119 (this could be changed to do that if you want).

@kingthorin
Copy link
Member

@kingthorin
Copy link
Member

Also needs a spotlessApply: ./gradlew :addOns:pscanrules:spotlessApply

@martinspielmann
Copy link
Contributor Author

Thanks @kingthorin and @thc202 for the hints, sorry saw that too late 🐒 Will update PR asap

The CWE id was already managed for these Alerts/ScanRules and within ZAP they were already presented to the user.
However, when visiting the alert detail pages online (https://www.zaproxy.org/docs/alerts/), CWE (and WASC) links were not generated correctly.

The reason is, that the id values were set inline to the alert.
For the script which generates the detail pages however, the getter methods for the ids have to be added publicly to the class.

Note: if this solves the issue, I'll vote for another follow up issue to add this change to the PluginPassiveScanner parent class,
so that the docs will be generated consistently over all Passive Scan Rules.

Fixes zaproxy/zaproxy#6140.

Signed-off-by: Martin Spielmann <[email protected]>
@martinspielmann martinspielmann force-pushed the 6140-expose-cwe-id-for-cookie-alerts branch from 06e2f7e to e8cf83b Compare September 3, 2020 14:08
@kingthorin
Copy link
Member

kingthorin commented Sep 3, 2020

No worries. Every project/contribution is a new adventure 😀

@martinspielmann
Copy link
Contributor Author

Regarding the comment:

It does, but there's a new way to provide the alert data, more details in zaproxy/zaproxy#6119 (this could be changed to do that if you want).

After reading through, I think it would indeed be cleaner to go for the new List<Alert> getExampleAlerts() method. Should I add that here already or would it be preferable to have that in another PR clearly referencing to the introduction of getExampleAlerts

@kingthorin
Copy link
Member

I'm fine with it being here. You can always squash this.

To comply with zaproxy/zaproxy#6140, the affected ScanRules have been provided with a getExampeAlerts method.
It allows maintenance scripts to generate documentation pages in a common way.

Signed-off-by: Martin Spielmann <[email protected]>
@martinspielmann
Copy link
Contributor Author

I'm fine with it being here. You can always squash this.

Ok ready :)

@thc202 thc202 changed the title Expose getCweId and getWascId method for Cookie scan rules Provide example alerts in Cookie scan rules Sep 7, 2020
Copy link
Member

@kingthorin kingthorin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor thing, and it’s an existing issue

Signed-off-by: Martin Spielmann <[email protected]>
Copy link
Member

@kingthorin kingthorin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@thc202 thc202 merged commit b69b2d9 into zaproxy:master Sep 7, 2020
@thc202
Copy link
Member

thc202 commented Sep 7, 2020

Thank you!

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.

Expose CWE Ids for cookie alerts
3 participants