Skip to content

Commit

Permalink
Process triggers and notifications also on exception during ActiveSca…
Browse files Browse the repository at this point in the history 
…ns sendAndReceive. Fixes zaproxy#7004

Signed-off-by: Dennis Kniep <kniepdennis@gmail.com>
  • Loading branch information
@denniskniep
denniskniep committed Jan 16, 2022
1 parent 0e49dcb commit 7f82c53
Show file tree
Hide file tree
Showing 22 changed files with 745 additions and 103 deletions.
24 changes: 18 additions & 6 deletions zap/src/main/java/org/parosproxy/paros/core/scanner/AbstractPlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@
// ZAP: 2020/11/17 Use new TechSet#getAllTech().
// ZAP: 2020/11/26 Use Log4j2 getLogger() and deprecate Log4j1.x.
// ZAP: 2021/07/20 Correct message updated with the scan rule ID header (Issue 6689).
// ZAP: 2022/01/04 Process notifications also on exception during sendAndReceive (Issue
// 7004).
package org.parosproxy.paros.core.scanner;

import java.io.IOException;
Expand All @@ -94,6 +96,7 @@
import org.parosproxy.paros.network.HttpStatusCode;
import org.zaproxy.zap.control.AddOn;
import org.zaproxy.zap.extension.anticsrf.ExtensionAntiCSRF;
import org.zaproxy.zap.extension.ascan.ScannerTaskResult;
import org.zaproxy.zap.extension.custompages.CustomPage;
import org.zaproxy.zap.model.Tech;
import org.zaproxy.zap.model.TechSet;
Expand Down Expand Up @@ -309,16 +312,25 @@ protected void sendAndReceive(
// ZAP: Runs the "beforeScan" methods of any ScannerHooks
parent.performScannerHookBeforeScan(message, this);

if (isFollowRedirect) {
parent.getHttpSender().sendAndReceive(message, getParent().getRedirectRequestConfig());
} else {
parent.getHttpSender().sendAndReceive(message, false);
try {
if (isFollowRedirect) {
parent.getHttpSender()
.sendAndReceive(message, getParent().getRedirectRequestConfig());
} else {
parent.getHttpSender().sendAndReceive(message, false);
}
} catch (IOException e) {
message.setErrorResponse(e);
// ZAP: Notify parent
parent.notifyNewMessage(this, new ScannerTaskResult(message, e.getLocalizedMessage()));
return;
}

// ZAP: Notify parent
parent.notifyNewMessage(this, message);
parent.notifyNewMessage(this, new ScannerTaskResult(message));

// ZAP: Set the history reference back and run the "afterScan" methods of any ScannerHooks
// ZAP: Set the history reference back and run the "afterScan" methods of any
// ScannerHooks
parent.performScannerHookAfterScan(message, this);
}

Expand Down
4 changes: 3 additions & 1 deletion zap/src/main/java/org/parosproxy/paros/core/scanner/Analyser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
// ZAP: 2019/06/05 Normalise format/style.
// ZAP: 2019/07/26 Remove null check in sendAndReceive(HttpMessage). (LGTM Issue)
// ZAP: 2020/11/26 Use Log4j 2 classes for logging.
// ZAP: 2022/01/04 Use changed ScannerListener interface
package org.parosproxy.paros.core.scanner;

import java.io.IOException;
Expand All @@ -63,6 +64,7 @@
import org.parosproxy.paros.network.HttpMessage;
import org.parosproxy.paros.network.HttpSender;
import org.parosproxy.paros.network.HttpStatusCode;
import org.zaproxy.zap.extension.ascan.ScannerTaskResult;
import org.zaproxy.zap.model.StructuralNode;

public class Analyser {
Expand Down Expand Up @@ -519,7 +521,7 @@ private void sendAndReceive(HttpMessage msg) throws HttpException, IOException {

httpSender.sendAndReceive(msg, parent.getRedirectRequestConfig());
requestCount++;
parent.notifyNewMessage(msg);
parent.notifyNewMessage(new ScannerTaskResult(msg));
}

public int getDelayInMs() {
Expand Down
40 changes: 36 additions & 4 deletions zap/src/main/java/org/parosproxy/paros/core/scanner/HostProcess.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@
// ZAP: 2020/11/26 Use Log4j 2 classes for logging.
// ZAP: 2021/09/14 No longer force single threading if Anti CSRF handling turned on.
// ZAP: 2021/09/30 Pass plugin to PluginStats instead of just the name.
// ZAP: 2022/01/04 Use changed ScannerListener interface
package org.parosproxy.paros.core.scanner;

import java.io.IOException;
Expand Down Expand Up @@ -126,6 +127,7 @@
import org.parosproxy.paros.network.HttpSender;
import org.zaproxy.zap.extension.alert.ExtensionAlert;
import org.zaproxy.zap.extension.ascan.ScanPolicy;
import org.zaproxy.zap.extension.ascan.ScannerTaskResult;
import org.zaproxy.zap.extension.ascan.filters.FilterResult;
import org.zaproxy.zap.extension.ascan.filters.ScanFilter;
import org.zaproxy.zap.extension.custompages.CustomPage;
Expand Down Expand Up @@ -646,7 +648,7 @@ private boolean scanMessage(Plugin plugin, int messageId) {
private boolean obtainResponse(HistoryReference hRef, HttpMessage message) {
try {
getHttpSender().sendAndReceive(message);
notifyNewMessage(message);
notifyNewMessage(new ScannerTaskResult(message));
requestCount++;
return true;
} catch (IOException e) {
Expand Down Expand Up @@ -839,9 +841,11 @@ private void notifyHostComplete() {
*
* @param msg the new HTTP message
* @since 1.2.0
* @deprecated (2.12.0) Use {@link #notifyNewMessage(ScannerTaskResult)}
*/
@Deprecated
public void notifyNewMessage(HttpMessage msg) {
parentScanner.notifyNewMessage(msg);
notifyNewMessage(new ScannerTaskResult(msg));
}

/**
Expand All @@ -852,9 +856,37 @@ public void notifyNewMessage(HttpMessage msg) {
* @throws IllegalArgumentException if the given {@code plugin} is {@code null}.
* @since 2.5.0
* @see #notifyNewMessage(Plugin)
* @deprecated (2.12.0) Use {@link #notifyNewMessage(Plugin, ScannerTaskResult)}
*/
@Deprecated
public void notifyNewMessage(Plugin plugin, HttpMessage message) {
parentScanner.notifyNewMessage(message);
notifyNewMessage(plugin, new ScannerTaskResult(message));
}

/**
* Notifies interested parties that a new message was sent (and received).
*
* <p>{@link Plugin Plugins} should call {@link #notifyNewMessage(Plugin)} or {@link
* #notifyNewMessage(Plugin, ScannerTaskResult)}, instead.
*
* @param scannerTaskResult contains the new HTTP message
* @since 1.2.0
*/
public void notifyNewMessage(ScannerTaskResult scannerTaskResult) {
parentScanner.notifyNewMessage(scannerTaskResult);
}

/**
* Notifies that the given {@code plugin} sent (and received) the given HTTP message.
*
* @param plugin the plugin that sent the message
* @param scannerTaskResult contains the message sent
* @throws IllegalArgumentException if the given {@code plugin} is {@code null}.
* @since 2.5.0
* @see #notifyNewMessage(Plugin)
*/
public void notifyNewMessage(Plugin plugin, ScannerTaskResult scannerTaskResult) {
parentScanner.notifyNewMessage(scannerTaskResult);
notifyNewMessage(plugin);
}

Expand All @@ -867,7 +899,7 @@ public void notifyNewMessage(Plugin plugin, HttpMessage message) {
* @param plugin the plugin that sent a non-HTTP message
* @throws IllegalArgumentException if the given parameter is {@code null}.
* @since 2.5.0
* @see #notifyNewMessage(Plugin, HttpMessage)
* @see #notifyNewMessage(Plugin, ScannerTaskResult)
*/
public void notifyNewMessage(Plugin plugin) {
if (plugin == null) {
Expand Down
10 changes: 9 additions & 1 deletion zap/src/main/java/org/parosproxy/paros/core/scanner/Scanner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
// ZAP: 2020/11/17 Use new TechSet#getAllTech().
// ZAP: 2020/11/26 Use Log4j 2 classes for logging.
// ZAP: 2021/05/14 Remove redundant type arguments.
// ZAP: 2022/01/04 Use changed ScannerListener interface
package org.parosproxy.paros.core.scanner;

import java.security.InvalidParameterException;
Expand Down Expand Up @@ -79,6 +80,7 @@
import org.parosproxy.paros.network.HttpMessage;
import org.zaproxy.zap.extension.ascan.ActiveScanEventPublisher;
import org.zaproxy.zap.extension.ascan.ScanPolicy;
import org.zaproxy.zap.extension.ascan.ScannerTaskResult;
import org.zaproxy.zap.extension.ascan.filters.ScanFilter;
import org.zaproxy.zap.extension.ruleconfig.RuleConfigParam;
import org.zaproxy.zap.extension.script.ScriptCollection;
Expand Down Expand Up @@ -455,10 +457,16 @@ public boolean isPaused() {
return pause;
}

/** @deprecated (2.12.0) Use {@link #notifyNewMessage(ScannerTaskResult)} */
@Deprecated
public void notifyNewMessage(HttpMessage msg) {
notifyNewMessage(new ScannerTaskResult(msg));
}

public void notifyNewMessage(ScannerTaskResult scannerTaskResult) {
for (int i = 0; i < listenerList.size(); i++) {
ScannerListener listener = listenerList.get(i);
listener.notifyNewMessage(msg);
listener.notifyNewTaskResult(scannerTaskResult);
}
}

Expand Down
11 changes: 9 additions & 2 deletions zap/src/main/java/org/parosproxy/paros/core/scanner/ScannerListener.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@
// ZAP: 2019/06/05 Normalise format/style.
// ZAP: 2019/12/10 Issue 5278: Adding filtered messages to active scan panel.
// ZAP: 2021/05/14 Remove empty statement.
// ZAP: 2022/01/14 deprecated notifyNewMessage and added notifyNewTaskResult
package org.parosproxy.paros.core.scanner;

import org.parosproxy.paros.network.HttpMessage;
import org.zaproxy.zap.extension.ascan.ScannerTaskResult;

public interface ScannerListener {

Expand All @@ -41,8 +43,13 @@ public interface ScannerListener {

void alertFound(Alert alert);

// ZAP: Added notifyNewMessage
void notifyNewMessage(HttpMessage msg);
/** @deprecated (2.12.0) Use {@link #notifyNewTaskResult(ScannerTaskResult)} */
@Deprecated
default void notifyNewMessage(HttpMessage msg) {}

default void notifyNewTaskResult(ScannerTaskResult scannerTaskResult) {
notifyNewMessage(scannerTaskResult.getHttpMessage());
}

/**
* Added to notify reason for filtering message from scanning.
Expand Down
55 changes: 55 additions & 0 deletions zap/src/main/java/org/parosproxy/paros/network/HttpMessage.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,12 @@
// ZAP: 2021/04/01 Detect WebSocket upgrade messages having multiple Connection directives
// ZAP: 2021/05/11 Fixed conversion of Request Method to/from CONNECT
// ZAP: 2021/05/14 Add missing override annotation.
// ZAP: 2022/01/14 Added setErrorResponse method

package org.parosproxy.paros.network;

import java.net.HttpCookie;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
Expand All @@ -74,11 +77,14 @@
import java.util.SortedSet;
import java.util.TreeSet;
import java.util.Vector;
import javax.net.ssl.SSLException;
import org.apache.commons.httpclient.URI;
import org.apache.commons.httpclient.URIException;
import org.apache.commons.lang.StringUtils;
import org.apache.commons.lang.exception.ExceptionUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.parosproxy.paros.Constant;
import org.parosproxy.paros.model.HistoryReference;
import org.parosproxy.paros.model.Model;
import org.zaproxy.zap.eventBus.Event;
Expand Down Expand Up @@ -1239,4 +1245,53 @@ public Map<String, String> toEventData() {
public String getType() {
return MESSAGE_TYPE;
}

public void setErrorResponse(Exception cause) {
StringBuilder strBuilder = new StringBuilder(250);
if (cause instanceof SSLException) {
strBuilder.append(Constant.messages.getString("network.ssl.error.connect"));
strBuilder.append(this.getRequestHeader().getURI().toString()).append('\n');
strBuilder
.append(Constant.messages.getString("network.ssl.error.exception"))
.append(cause.getMessage())
.append('\n');
strBuilder
.append(Constant.messages.getString("network.ssl.error.exception.rootcause"))
.append(ExceptionUtils.getRootCauseMessage(cause))
.append('\n');
strBuilder.append(
Constant.messages.getString(
"network.ssl.error.help",
Constant.messages.getString("network.ssl.error.help.url")));

strBuilder.append("\n\nStack Trace:\n");
for (String stackTraceFrame : ExceptionUtils.getRootCauseStackTrace(cause)) {
strBuilder.append(stackTraceFrame).append('\n');
}
} else {
strBuilder
.append(cause.getClass().getName())
.append(": ")
.append(cause.getLocalizedMessage())
.append("\n\nStack Trace:\n");
for (String stackTraceFrame : ExceptionUtils.getRootCauseStackTrace(cause)) {
strBuilder.append(stackTraceFrame).append('\n');
}
}

String message = strBuilder.toString();

HttpResponseHeader responseHeader;
try {
responseHeader = new HttpResponseHeader("HTTP/1.1 400 ZAP IO Error");
responseHeader.setHeader(HttpHeader.CONTENT_TYPE, "text/plain; charset=UTF-8");
responseHeader.setHeader(
HttpHeader.CONTENT_LENGTH,
Integer.toString(message.getBytes(StandardCharsets.UTF_8).length));
this.setResponseHeader(responseHeader);
this.setResponseBody(message);
} catch (HttpMalformedHeaderException e) {
log.error("Failed to create error response:", e);
}
}
}
38 changes: 32 additions & 6 deletions zap/src/main/java/org/zaproxy/zap/extension/ascan/ActiveScan.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ public static enum State {
private int maxResultsToList = 0;

private final List<Integer> hRefs = Collections.synchronizedList(new ArrayList<>());
private final List<Integer> hRefsWithError = Collections.synchronizedList(new ArrayList<>());
private final List<Integer> alerts = Collections.synchronizedList(new ArrayList<>());

private ScheduledExecutorService scheduler;
Expand Down Expand Up @@ -273,7 +274,8 @@ public ActiveScanTableModel getMessagesTableModel() {
}

@Override
public void notifyNewMessage(final HttpMessage msg) {
public void notifyNewTaskResult(final ScannerTaskResult scannerTaskResult) {
HttpMessage msg = scannerTaskResult.getHttpMessage();
HistoryReference hRef = msg.getHistoryRef();
if (hRef == null) {
try {
Expand All @@ -283,12 +285,12 @@ public void notifyNewMessage(final HttpMessage msg) {
HistoryReference.TYPE_SCANNER_TEMPORARY,
msg);
msg.setHistoryRef(null);
hRefs.add(hRef.getHistoryId());
addHref(hRef.getHistoryId(), scannerTaskResult.isProcessed());
} catch (HttpMalformedHeaderException | DatabaseException e) {
log.error(e.getMessage(), e);
}
} else {
hRefs.add(hRef.getHistoryId());
addHref(hRef.getHistoryId(), scannerTaskResult.isProcessed());
}

this.rcTotals.incResponseCodeCount(msg.getResponseHeader().getStatusCode());
Expand All @@ -299,17 +301,26 @@ public void notifyNewMessage(final HttpMessage msg) {
if (this.rcTotals.getTotal() > this.maxResultsToList) {
removeFirstHistoryReferenceInEdt();
}
addHistoryReferenceInEdt(hRef);
addHistoryReferenceInEdt(hRef, scannerTaskResult);
}
}

private void addHistoryReferenceInEdt(final HistoryReference hRef) {
private void addHref(int historyId, boolean noError){
if(noError){
hRefs.add(historyId);
} else{
hRefsWithError.add(historyId);
}
}

private void addHistoryReferenceInEdt(
final HistoryReference hRef, ScannerTaskResult scannerTaskResult) {
EventQueue.invokeLater(
new Runnable() {

@Override
public void run() {
messagesTableModel.addHistoryReference(hRef);
messagesTableModel.addEntry(hRef, scannerTaskResult);
}
});
}
Expand Down Expand Up @@ -374,6 +385,21 @@ public List<Integer> getMessagesIds() {
return hRefs;
}

/**
* Returns the IDs of all messages sent/created during the scan which threw an error (e.g. IOException). The message must be recreated
* with a HistoryReference.
*
* <p><strong>Note:</strong> Iterations must be {@code synchronized} on returned object. Failing
* to do so might result in {@code ConcurrentModificationException}.
*
* @return the IDs of all the messages sent/created during the scan which threw an error (e.g. IOException)
* @see HistoryReference
* @see ConcurrentModificationException
*/
public List<Integer> getMessagesIdsWithError() {
return hRefsWithError;
}

/**
* Returns the IDs of all alerts raised during the scan.
*
Expand Down
Loading

0 comments on commit 7f82c53

Please sign in to comment.