Skip to content

Conversation

Brandosp
Copy link

@Brandosp Brandosp commented Oct 1, 2025

Overview

Added example alerts for InsecureHttpMethodScanRule.

Related Issues

6119

Checklist

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

for more details, please refer to the developer rules and guidelines

Copy link

github-actions bot commented Oct 1, 2025

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@Brandosp
Copy link
Author

Brandosp commented Oct 1, 2025

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

@psiinon
Copy link
Member

psiinon commented Oct 1, 2025

Logo
Checkmarx One – Scan Summary & Detailsd304af45-d041-49bb-8373-64783e6bf326

Great job! No new security vulnerabilities introduced in this pull request


Use @Checkmarx to reach out to us for assistance.

Just send a PR comment with @Checkmarx followed by a natural language request.

Examples: @Checkmarx how are you able to help me? @Checkmarx rescan this PR

@kingthorin
Copy link
Member

This should cover all the different alerts the rule might raise. Including adding alertRefs (see issue zaproxy/zaproxy#7100 for more on that).

Added example alert functionality

Removed unnecessary import

Adheres to google-java style

Unnecessary exception

Applied spotlessApply

Signed-off-by: Brandosp <[email protected]>

Added more examples and added Alert refs

Signed-off-by: Brandosp <[email protected]>
Comment on lines +626 to +646
private AlertBuilder buildAlert(
String vulnName,
String vulnDesc,
String extraInfo,
String evidence,
HttpMessage msg,
VulnType currentVT) {
return newAlert()
.setConfidence(Alert.CONFIDENCE_MEDIUM)
.setName(
Constant.messages.getString(
"ascanbeta.insecurehttpmethod.detailed.name", vulnName))
.setDescription(vulnDesc)
.setOtherInfo(extraInfo)
.setSolution(Constant.messages.getString("ascanbeta.insecurehttpmethod.soln"))
.setEvidence(evidence)
.setMessage(msg)
.setCweId(getCweId())
.setWascId(getWascId())
.setAlertRef(getId() + "-" + currentVT.getRef());
}
Copy link
Member

@kingthorin kingthorin Oct 2, 2025

Choose a reason for hiding this comment

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

The builder should be usable by the existing code. Ex: Where alerts are currently raised it should be able to call the builder method(s) (there can be multiple if necessary) then just raise, like: buildAlert(....).raise();

Comment on lines +660 to +663
name = "DELETE Method Enabled";
description = "The server allows the DELETE HTTP method which can be unsafe.";
extraInfo = "DELETE requests could remove resources if exploited.";
evidence = "DELETE";
Copy link
Member

Choose a reason for hiding this comment

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

These values should be the actual values used in the alerts, though you may need to provide the details or specifics like "DELETE". You could also include these int he enum members as that likely simplifies things. It also ensure that they're translated etc.

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.

3 participants