NotificationsYou must be signed in to change notification settings
Fork2.5k
Star14.4k

Process triggers and notifications also on exception during ActiveScans sendAndReceive.#7005

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Open

denniskniep wants to merge1 commit intozaproxy:main

base:main

Choose a base branch

fromdenniskniep:feature/active-scan-process-notification-on-error

Open

Process triggers and notifications also on exception during ActiveScans sendAndReceive.#7005

denniskniep wants to merge1 commit intozaproxy:mainfromdenniskniep:feature/active-scan-process-notification-on-error

Conversation

Copy link

Member

denniskniep commentedJan 4, 2022•
edited by thc202
Loading

The results in the Active Scan panel will look like this:

Signed-off-by: Dennis Kniepkniepdennis@gmail.com

Fixes#7004

denniskniep force-pushed thefeature/active-scan-process-notification-on-error branch from2adde5a to8808e34Compare

January 4, 2022 08:36

thc202 added the Type-Task label

Jan 4, 2022

Copy link

MemberAuthor

denniskniep commentedJan 4, 2022•
edited
Loading

To be discussed:

process triggers and notifications only on specific ExceptionTypes:SocketTimeoutException,IOException ?
Set the exception/error message as response body?

Copy link

Member

kingthorin commentedJan 4, 2022

I'd be good with the exceptions being made into the response body (that's done elsewhere IIRC). I wonder if this should be tied to an option though and default off (to maintain closest to current behavior)?

Copy link

MemberAuthor

denniskniep commentedJan 5, 2022•
edited
Loading

Discussion from IRC:

have a visual indication - just be an icon for the ones with errors (like the spider has with the red one)
add exceptions to the response body
provide messages with errors separately, at least for the API/programmatically (way easier to inspect them that way rather than go through thousands of requests)

denniskniep changed the title~~Process triggers and notifications also on exception during ActiveScans sendAndReceive.~~[WIP] Process triggers and notifications also on exception during ActiveScans sendAndReceive.

Jan 5, 2022

Copy link

MemberAuthor

denniskniep commentedJan 5, 2022

@thc202 can you provide more Context regarding

provide messages with errors separately, at least for the API/programmaticall

Is it only separating for API (/ascan/view/messagesIds/)

Copy link

Member

thc202 commentedJan 10, 2022•
edited
Loading

For the code and the ZAP API, in theActiveScan expose a method that returns a list with the IDs of the messages with errors (similar toSpiderScan which exposes successful and error messages separately). In the ZAP API another endpoint to get those requests.

thc202 reviewed

Jan 10, 2022

View reviewed changes

zap/src/main/java/org/parosproxy/paros/core/scanner/AbstractPlugin.java Outdated

		}
		}finally {
		// ZAP: Notify parent
		parent.notifyNewMessage(this,message);

Copy link

Member

thc202Jan 10, 2022

Choose a reason for hiding this comment

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

Might be better just notify onIOExceptions.

zap/src/main/java/org/parosproxy/paros/core/scanner/AbstractPlugin.java Outdated

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

Copy link

Member

thc202Jan 10, 2022

Choose a reason for hiding this comment

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

Since we didn't have a request for this I'd keep the previous behaviour.

denniskniep force-pushed thefeature/active-scan-process-notification-on-error branch from8808e34 to0d4ba97Compare

January 14, 2022 00:14

denniskniep commented

Jan 14, 2022

View reviewed changes

zap/src/main/java/org/parosproxy/paros/core/scanner/AbstractPlugin.java Outdated

		parent.getHttpSender().sendAndReceive(message,false);
		}
		}catch (IOExceptione) {
		// TODO: How to update HistoryRef with error Response?

Copy link

MemberAuthor

denniskniepJan 14, 2022

Choose a reason for hiding this comment

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

@thc202 can you give me any hint here?
How can I update the HistoryRef with error Response? In case HttpMessage has already a HistoryRef and is attached to the Alert

Copy link

MemberAuthor

denniskniepJan 14, 2022

Choose a reason for hiding this comment

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

Discussed in IRC: No workarounds here. Problem is releated to oast
I guess the way how histRef ist set to HttpMessage is like the following:
.alertFound(alert, null) is triggered by oast here before exception is catched:

https://github.com/zaproxy/zap-extensions/blob/7016bdf3607f31f42ceb3206c86b437131b0452b/addOns/oast/src/main/java/org/zaproxy/addon/oast/ExtensionOast.java#L268

And then a HistRef is created

zaproxy/zap/src/main/java/org/zaproxy/zap/extension/alert/ExtensionAlert.java

Line 189 inf68b04e

newHistoryReference(

Which sets automatically the histref member on the Httpmessage

Resulting in that the Histref is set to the message not including the error message which is later set

we should change oast to allow to manipulate the alert/message before caching it

denniskniep force-pushed thefeature/active-scan-process-notification-on-error branch 2 times, most recently fromb2f6be5 to7f82c53Compare

January 16, 2022 22:11

Process triggers and notifications also on exception during ActiveSca…

e965de5

…ns sendAndReceive.Fixeszaproxy#7004Signed-off-by: Dennis Kniep <kniepdennis@gmail.com>

denniskniep force-pushed thefeature/active-scan-process-notification-on-error branch from7f82c53 toe965de5Compare

January 16, 2022 22:13

denniskniep changed the title~~[WIP] Process triggers and notifications also on exception during ActiveScans sendAndReceive.~~Process triggers and notifications also on exception during ActiveScans sendAndReceive.

Jan 16, 2022

Copy link

MemberAuthor

denniskniep commentedJan 27, 2022

Anything else to do here before merge ?

psiinon reviewed

Jan 27, 2022

View reviewed changes

zap/src/main/java/org/zaproxy/zap/extension/callback/ui/CustomColumn.java

		packageorg.zaproxy.zap.extension.callback.ui;

		/** @deprecated (2.11.0)Superseded by the OAST add-on. */
		/** @deprecated (2.11.0){@link org.zaproxy.zap.view.table.CustomColumn}. */

Copy link

Member

psiinonJan 27, 2022

Choose a reason for hiding this comment

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

Is this intensional? If so why?

Copy link

MemberAuthor

denniskniepJan 27, 2022

Choose a reason for hiding this comment

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

This was deprecated once due to moving Oast addon from ZAP to ZAP-Extension.

But I used the original class again. Therefore its there again but within an other package. I thought this info could be useful to know if someone stumbles upon that deprecation

Copy link

Member

psiinonFeb 7, 2022

Choose a reason for hiding this comment

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

ok, find by me :)

psiinon reviewed

Jan 27, 2022

View reviewed changes

...va/org/zaproxy/zap/extension/callback/ui/DefaultCustomColumnHistoryReferencesTableModel.java

		importorg.zaproxy.zap.view.table.DefaultHistoryReferencesTableEntry;
		importorg.zaproxy.zap.view.table.DefaultHistoryReferencesTableModel;

		/** @deprecated (2.11.0) Superseded by the OAST add-on. */

Copy link

Member

psiinonJan 27, 2022

Choose a reason for hiding this comment

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

Ditto

Labels

Type-Task

Movatterモバイル変換

Process triggers and notifications also on exception during ActiveScans sendAndReceive.#7005

Are you sure you want to change the base?

Process triggers and notifications also on exception during ActiveScans sendAndReceive.#7005

Uh oh!

Conversation

denniskniep commentedJan 4, 2022• edited by thc202Loading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

denniskniep commentedJan 4, 2022• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

kingthorin commentedJan 4, 2022

Uh oh!

denniskniep commentedJan 5, 2022• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

denniskniep commentedJan 5, 2022

Uh oh!

thc202 commentedJan 10, 2022• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

denniskniep commentedJan 27, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

denniskniep commentedJan 4, 2022•
edited by thc202
Loading

denniskniep commentedJan 4, 2022•
edited
Loading

denniskniep commentedJan 5, 2022•
edited
Loading

thc202 commentedJan 10, 2022•
edited
Loading