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.

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

ascanrules: address FPs in scan rule 20017 #5730

Merged
merged 1 commit into from
Sep 17, 2024
Merged

ascanrules: address FPs in scan rule 20017 #5730

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions addOns/ascanrules/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
- Maintenance changes.
- The Spring Actuator Scan Rule now includes example alert functionality for documentation generation purposes (Issue 6119).

### Fixed
- Address false positives with Source Code Disclosure - CVE-2012-1823 scan rule, by not scanning binary responses and responses that already contain PHP source (Issue 8638).

## [67] - 2024-07-22

### Changed
Expand Down
14 changes: 13 additions & 1 deletion ...in/java/org/zaproxy/zap/extension/ascanrules/SourceCodeDisclosureCve20121823ScanRule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,18 @@ public String getReference() {
public void scan() {
try {

if (!getBaseMsg().getResponseHeader().isText()) {
if (!getBaseMsg().getResponseHeader().isText()
|| ResourceIdentificationUtils.responseContainsControlChars(getBaseMsg())) {
return; // Ignore images, pdfs, etc.
}
if (getAlertThreshold() != AlertThreshold.LOW
&& ResourceIdentificationUtils.isJavaScript(getBaseMsg())) {
return;
}

if (isEvidenceInOriginalResponse()) {
return;
}
// at Low or Medium strength, do not attack URLs which returned "Not Found"
AttackStrength attackStrength = getAttackStrength();
if ((attackStrength == AttackStrength.LOW || attackStrength == AttackStrength.MEDIUM)
Expand Down Expand Up @@ -181,6 +186,13 @@ public void scan() {
}
}

private boolean isEvidenceInOriginalResponse() {
var response = getBaseMsg().getResponseBody().toString();
String responseBodyDecoded = new Source(response).getRenderer().toString();
return PHP_PATTERN1.matcher(responseBodyDecoded).matches()
|| PHP_PATTERN2.matcher(responseBodyDecoded).matches();
}

private AlertBuilder buildAlert(String otherInfo) {
return newAlert()
.setConfidence(Alert.CONFIDENCE_MEDIUM)
Expand Down
32 changes: 32 additions & 0 deletions ...org/zaproxy/zap/extension/ascanrules/SourceCodeDisclosureCve20121823ScanRuleUnitTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import java.util.Map;
import org.apache.commons.text.StringEscapeUtils;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.parosproxy.paros.core.scanner.Alert;
import org.parosproxy.paros.core.scanner.Plugin.AlertThreshold;
import org.parosproxy.paros.core.scanner.Plugin.AttackStrength;
Expand Down Expand Up @@ -253,6 +255,36 @@ protected Response serve(IHTTPSession session) {
assertThat(alertsRaised, hasSize(0));
}

@ParameterizedTest
@ValueSource(strings = {PHP_SOURCE_TAGS, PHP_SOURCE_ECHO_TAG})
void shouldNotScanIfPhpSourceWasAlreadyPresentInResponse(String source) throws Exception {
// Given
var response =
"<html><body>PHP Tutorial: <code>"
+ StringEscapeUtils.escapeHtml4(source)
+ "</code></body></html>";
HttpMessage message = getHttpMessage("GET", "/", response);
rule.init(message, parent);
// When
rule.scan();
// Then
assertThat(httpMessagesSent, hasSize(0));
assertThat(alertsRaised, hasSize(0));
}

@Test
void shouldNotScanBinaryResponse() throws Exception {
// Given
var response = "�PNG\n\n";
HttpMessage message = getHttpMessage("GET", "/", response);
rule.init(message, parent);
// When
rule.scan();
// Then
assertThat(httpMessagesSent, hasSize(0));
assertThat(alertsRaised, hasSize(0));
}

@Test
void shouldAlertIfPhpEchoTagsWereDisclosedInResponseBody() throws Exception {
// Given
Expand Down