Skip to content

ascanrules: XPath Injection add Custom Payloads support #6465

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

ascanrules: XPath Injection add Custom Payloads support #6465

wants to merge 1 commit into from

Conversation

kingthorin
Copy link
Member

@kingthorin kingthorin commented May 25, 2025 •
edited
Loading

Overview

ascanrules

  • CHANGELOG > Add change note.
  • ExtensionPayloader > Add appropriate hooks etc.
  • ExtensionPayloaderUnitTest > Assert hook/unload behavior.
  • ascanrules.html > Update help entry.
  • XpathInjectionScanRule > Update functionality to support Custom Payloads.

custompayloads:

  • CHANGELOG > Add note.
  • PayloadCategory > Add support for adding payloads which are initially disabled.

Related Issues

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

@psiinon
Copy link
Member

psiinon commented May 26, 2025 •
edited
Loading

Logo
Checkmarx One – Scan Summary & Details – 80e58ac5-fa62-42a3-8807-e8330c7eb5d5

Great job, no security vulnerabilities found in this Pull Request

@kingthorin kingthorin force-pushed the cp-moar branch 2 times, most recently from 9243fc4 to be9bae2 Compare May 26, 2025 13:38
thc202
thc202 reviewed May 26, 2025
View reviewed changes
...s/custompayloads/src/main/java/org/zaproxy/zap/extension/custompayloads/PayloadCategory.java Outdated
return createPayloads(name, payloads, true);
}

@SuppressWarnings("unused")
Copy link
Member

Choose a reason for hiding this comment

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

Seems used, it would also be better if lists were not created when they are not needed.

@kingthorin kingthorin force-pushed the cp-moar branch 6 times, most recently from 527c821 to 75a1205 Compare May 28, 2025 09:59
@kingthorin
ascanrules: XPath Injection add Custom Payloads support
23ffcec 
ascanrules
- CHANGELOG > Add change note.
- ExtensionPayloader > Add appropriate hooks etc.
- ExtensionPayloaderUnitTest > Assert hook/unload behavior.
- ascanrules.html > Update help entry.
- XpathInjectionScanRule > Update functionality to support Custom
Payloads.

custompayloads:
- CHANGELOG > Add note.
- PayloadCategory > Add support for adding payloads which are initially
disabled.

Signed-off-by: kingthorin <[email protected]>
thc202
thc202 reviewed Jun 2, 2025
View reviewed changes
addOns/ascanrules/ascanrules.gradle.kts
@@ -32,7 +32,7 @@ zapAddOn {
dependencies {
addOns {
register("custompayloads") {
version.set(">= 0.9.0 & < 1.0.0")
version.set(">= 0.15.0 & < 1.0.0")
Copy link
Member

Choose a reason for hiding this comment

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

Update the existing entry to also mention this add-on, while optional we should not release features that will not work.

...ns/ascanrules/src/main/java/org/zaproxy/zap/extension/ascanrules/XpathInjectionScanRule.java
return errorProvider;
}

public static void setPayloadProvider(Supplier<Iterable<String>> provider) {
Copy link
Member

Choose a reason for hiding this comment

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

Would be good to test this.

thc202
thc202 reviewed Jun 2, 2025
View reviewed changes
...s/custompayloads/src/main/java/org/zaproxy/zap/extension/custompayloads/PayloadCategory.java
}

List<CustomPayload> defaultPayloads = new ArrayList<>(payloads.size());
List<CustomPayload> addedPayloads = new ArrayList<>(payloads.size());
Copy link
Member

@thc202 thc202 Jun 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Why rename this? They are still default payloads. Also, the name createNonDefaultPayloads is misleading, they are default payloads but disabled. (i.e. defaultPayloads.addAll(createNonDefaultPayloads why are we adding non-default payloads to the default list?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thc202 thc202 thc202 left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for Custom Payloads in XPath Injection scan rule for detection of uncommon errors
3 participants
@kingthorin @psiinon @thc202