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.

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

selenium: add open custom request in browser feature #4155

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ArkaprabhaChakraborty
Copy link
Contributor

Related to: zaproxy/zaproxy#1456
Signed-off-by: ArkaprabhaChakraborty chakrabortyarkaprabha998@gmail.com

@thc202 thc202 changed the title Add open custom request in browser feature selenium: add open custom request in browser feature Oct 20, 2022
@ArkaprabhaChakraborty
Copy link
Contributor Author

ArkaprabhaChakraborty commented Oct 20, 2022

Wait ... now why did Java 18 fail?
Edit: A warning is failing the job :)

@ArkaprabhaChakraborty
Copy link
Contributor Author

ArkaprabhaChakraborty commented Oct 20, 2022

Found this. Probably won't be a wise idea to integrate it with ZAP tho... @thc202 can you please drop the ideas/headstart here?

@kingthorin
Copy link
Member

You need to suppress the serialization thing in some UI classes. Search PRs a bunch were done at one or two points.

@ArkaprabhaChakraborty
Copy link
Contributor Author

Got it... Apparently its there in this extension itself :).....

Copy link
Contributor Author

@ArkaprabhaChakraborty ArkaprabhaChakraborty left a comment

Choose a reason for hiding this comment

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

Discussions regarding PopupMenuItemOpenCustomRequestInBrowser :)

@ArkaprabhaChakraborty ArkaprabhaChakraborty force-pushed the arkaprabha/add-custom-request-response-browser branch from 848061d to 7d11572 Compare October 26, 2022 11:49
@ArkaprabhaChakraborty
Copy link
Contributor Author

this can be tried out now :)... just needs unit tests and help docs for completion.

@kingthorin
Copy link
Member

kingthorin commented Jan 6, 2023

Do you need advice on local testing or something? Is there a way we can help you cut down on pushes of small discrete changes?

@thc202
Copy link
Member

thc202 commented Jan 6, 2023

It was passing locally but failing in CI.

@kingthorin
Copy link
Member

Oh okay.

@ArkaprabhaChakraborty
Copy link
Contributor Author

Ahh finally :)

@ArkaprabhaChakraborty ArkaprabhaChakraborty force-pushed the arkaprabha/add-custom-request-response-browser branch from ad38613 to cd4513b Compare January 7, 2023 13:27
Helps in viewing a custom request with a browser.

Signed-off-by: ArkaprabhaChakraborty <chakrabortyarkaprabha998@gmail.com>
@ArkaprabhaChakraborty ArkaprabhaChakraborty force-pushed the arkaprabha/add-custom-request-response-browser branch from cd4513b to 5131bda Compare January 7, 2023 13:42
@ArkaprabhaChakraborty
Copy link
Contributor Author

ping @thc202 @kingthorin @psiinon :)
reason: impatience :)

@kingthorin
Copy link
Member

I appreciate your honesty 😀

@kingthorin
Copy link
Member

Code seems okay to me. I'm not sure I follow the force https part. I'll try to build this branch tonight and test it.

@thc202
Copy link
Member

thc202 commented Jan 18, 2023

The changelog and help should be updated.

}
return null;
} catch (Exception e) {
throw new ApiException(ApiException.Type.URL_NOT_FOUND, e);
Copy link
Member

Choose a reason for hiding this comment

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

I ended up with a ton of 1286162 [ZAP-IO-Server-1-17] WARN org.zaproxy.zap.extension.api.API - Request to callback URL https://demo.testfire.net/zapCallBackUrl/images/home2.jpg from 127.0.0.1 not found - this could be a callback url from a previous session or possibly an attempt to attack ZAP while testing. I guess this is due to media included in the custom request I was opening? It's awfully chatty/loggy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't understand how you got this :)..... the hist param didn't get appended :).....

Comment on lines +66 to +67
} catch (Exception e) {
View.getSingleton().showWarningDialog(e.getMessage());
Copy link
Member

Choose a reason for hiding this comment

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

I seemed to hit the exception case more often than not.

I browsed to demo.testfire.net clicked a few pages, submitted the login POST with any old values. Then tried to open a few of the things via the new menu.

I don't really understand what's "Custom" about it, but simple GETs seemed to work while other GETs failed, and the one POST I had to work with never worked.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay so for the POST requests, I think we need to follow the redirect/"follow the lead" to the very next GET request. Actually, I guess this was the same concern in the issue itself. I did need some views on what could be done. The main "concern" was to send modified headers and load the "result" in a browser to kind of verify/view the exploit/vulnerability.

@kingthorin
Copy link
Member

kingthorin commented Jan 19, 2023

I think I'm failing to grasp what this is supposed to be doing. I see the URL being replaced partially by a built callback URL but the content loaded is sometimes/generally the URL/request I've asked for.

For example if I "Open Custom Request in Browser" from the Sites Tree against http://demo.testfire.net/login.jsp in Firefox the page is displayed and tells me my Login failed 🤷. So it seems to be loading the right page but the display in the address bar is https://demo.testfire.net/zapCallBackUrl/-8942414862249798826?hist=17&null.

@ArkaprabhaChakraborty
Copy link
Contributor Author

I think I'm failing to grasp what this is supposed to be doing. I see the URL being replaced partially by a built callback URL but the content loaded is sometimes/generally the URL/request I've asked for.

For example if I "Open Custom Request in Browser" from the Sites Tree against http://demo.testfire.net/login.jsp in Firefox the page is displayed and tells me my Login failed shrug. So it seems to be loading the right page but the display in the address bar is https://demo.testfire.net/zapCallBackUrl/-8942414862249798826?hist=17&null.

Did you try it with some new headers or cookies? :)

@ArkaprabhaChakraborty
Copy link
Contributor Author

ArkaprabhaChakraborty commented Jan 19, 2023

Most of the GETs that have a status 200 in the requestor/ request/response can be opened by this option. The fundamental difference is that this option can send in "modified" headers and cookies which is not quite possible for a normal browser. Also for "cache poisoning" attacks involving the usage of a header/cookie as a cache buster and a header/cookie as a vulnerable parameter... simply loading the URL using a browser doesn't help as it doesn't load the isolated vulnerable PoC which has been created in the cache with those cache busters... kinda helpful if we don't wanna poison the entire "base" cache response to which normal users are usually directed :P

@kingthorin
Copy link
Member

Did you try it with some new headers or cookies? :)

Added when/how? I’m not prompted. Like I said I dunno how this is meant to work.

@ArkaprabhaChakraborty
Copy link
Contributor Author

Did you try it with some new headers or cookies? :)

Added when/how? I’m not prompted. Like I said I dunno how this is meant to work.

In the requestor... simply modifying it? :)

@kingthorin
Copy link
Member

kingthorin commented Jan 20, 2023

Please tell me how users are meant to use this.

I’m currently accessing it from the Sites Tree. If I have to use Requestor first I’m going to send the request from Requestor how is that going to have an impact on my use of this?

I’m so confused and this is seeming VERY un-intuitive.

@ArkaprabhaChakraborty
Copy link
Contributor Author

Please tell me how users are meant to use this.

I’m currently accessing it from the Sites Tree. If I have to use Requestor first I’m going to send the request from Requestor how is that going to have an impact on my use of this?

I’m so confused and this is seeming VERY un-intuitive.

You don't have to use the "send". Just to modify the request I mean.. I have to check the cases/ GET requests for which it's failing and decide a method for POST requests.

@kingthorin
Copy link
Member

I'm still missing something here. I'll try to get time over the weekend to do a screen grab so that you can see how it's behaving for me.

@ArkaprabhaChakraborty
Copy link
Contributor Author

I'm still missing something here. I'll try to get time over the weekend to do a screen grab so that you can see how it's behaving for me.

Had a talk with @kingthorin :). It didn't misbehave anymore :P. The POST request works well, tried it out on three CTF and got the flags with ease :).

@ArkaprabhaChakraborty
Copy link
Contributor Author

Okay upon testing with http://demo.testfire.net/login.jsp The login request was a POST request, with the following:

Req Headers

POST http://demo.testfire.net/doLogin HTTP/1.1
Host: demo.testfire.net
User-Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:109.0) Gecko/20100101 Firefox/110.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/avif,image/webp,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Content-Type: application/x-www-form-urlencoded
Content-Length: 85
Origin: http://demo.testfire.net
Connection: keep-alive
Referer: http://demo.testfire.net/login.jsp
Cookie: JSESSIONID=C795E2A4E041568C1B8D7FC33A325FC3
Upgrade-Insecure-Requests: 1

Req Body

uid=1%3D1%27+SELECT+*+from+tables&passw=1%3D1%27+SELECT+*+from+tables&btnSubmit=Login

Response Header

HTTP/1.1 302 Found
Server: Apache-Coyote/1.1
Location: login.jsp
Content-Length: 0
Date: Sat, 25 Feb 2023 18:44:03 GMT

And a big breaking error
Screenshot from 2023-02-26 00-15-13

@ArkaprabhaChakraborty
Copy link
Contributor Author

Probably this mess-up is happening because of the moved temporarily and it's followups :)

@ArkaprabhaChakraborty
Copy link
Contributor Author

293964 [ZAP-IO-Server-1-3] WARN  org.zaproxy.zap.extension.api.API - Request to callback URL https://demo.testfire.net/zapCallBackUrl/login.jsp from 127.0.0.1 not found - this could be a callback url from a previous session or possibly an attempt to attack ZAP
1678379473564   Marionette      WARN    Ignoring event 'pageshow' because document has an invalid readyState of 'uninitialized'.
306327 [Thread-17] ERROR org.zaproxy.zap.extension.selenium.PopupMenuItemOpenCustomRequestInBrowser - Reached error page: about:neterror?e=nssFailure2&u=https%3A//demo.testfire.net/zapCallBackUrl/login.jsp&c=UTF-8&d=The%20connection%20to%20the%20server%20was%20reset%20while%20the%20page%20was%20loading.
Build info: version: 'unknown', revision: 'unknown', time: 'unknown'
System info: host: 'homer', ip: '127.0.1.1', os.name: 'Linux', os.arch: 'amd64', os.version: '5.15.0-67-generic', java.version: '17.0.6'
Driver info: org.openqa.selenium.firefox.FirefoxDriver
Capabilities {acceptInsecureCerts: true, browserName: firefox, browserVersion: 110.0.1, javascriptEnabled: true, moz:accessibilityChecks: false, moz:buildID: 20230227191043, moz:geckodriverVersion: 0.32.2, moz:headless: false, moz:platformVersion: 5.15.0-67-generic, moz:processID: 92527, moz:profile: /tmp/rust_mozprofilei9TI2p, moz:shutdownTimeout: 60000, moz:useNonSpecCompliantPointerOrigin: false, moz:webdriverClick: true, moz:windowless: false, pageLoadStrategy: normal, platform: LINUX, platformName: LINUX, proxy: Proxy(), setWindowRect: true, strictFileInteractability: false, timeouts: {implicit: 0, pageLoad: 300000, script: 30000}, unhandledPromptBehavior: dismiss and notify}
Session ID: 695162cb-6353-4d09-a770-6bb2cd12204b
org.openqa.selenium.WebDriverException: Reached error page: about:neterror?e=nssFailure2&u=https%3A//demo.testfire.net/zapCallBackUrl/login.jsp&c=UTF-8&d=The%20connection%20to%20the%20server%20was%20reset%20while%20the%20page%20was%20loading.
Build info: version: 'unknown', revision: 'unknown', time: 'unknown'
System info: host: 'homer', ip: '127.0.1.1', os.name: 'Linux', os.arch: 'amd64', os.version: '5.15.0-67-generic', java.version: '17.0.6'
Driver info: org.openqa.selenium.firefox.FirefoxDriver
Capabilities {acceptInsecureCerts: true, browserName: firefox, browserVersion: 110.0.1, javascriptEnabled: true, moz:accessibilityChecks: false, moz:buildID: 20230227191043, moz:geckodriverVersion: 0.32.2, moz:headless: false, moz:platformVersion: 5.15.0-67-generic, moz:processID: 92527, moz:profile: /tmp/rust_mozprofilei9TI2p, moz:shutdownTimeout: 60000, moz:useNonSpecCompliantPointerOrigin: false, moz:webdriverClick: true, moz:windowless: false, pageLoadStrategy: normal, platform: LINUX, platformName: LINUX, proxy: Proxy(), setWindowRect: true, strictFileInteractability: false, timeouts: {implicit: 0, pageLoad: 300000, script: 30000}, unhandledPromptBehavior: dismiss and notify}
Session ID: 695162cb-6353-4d09-a770-6bb2cd12204b
        at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method) ~[?:?]
        at jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77) ~[?:?]
        at jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45) ~[?:?]
        at java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499) ~[?:?]
        at java.lang.reflect.Constructor.newInstance(Constructor.java:480) ~[?:?]
        at org.openqa.selenium.remote.http.W3CHttpResponseCodec.createException(W3CHttpResponseCodec.java:187) ~[?:?]
        at org.openqa.selenium.remote.http.W3CHttpResponseCodec.decode(W3CHttpResponseCodec.java:122) ~[?:?]
        at org.openqa.selenium.remote.http.W3CHttpResponseCodec.decode(W3CHttpResponseCodec.java:49) ~[?:?]
        at org.openqa.selenium.remote.HttpCommandExecutor.execute(HttpCommandExecutor.java:158) ~[?:?]
        at org.openqa.selenium.remote.service.DriverCommandExecutor.execute(DriverCommandExecutor.java:83) ~[?:?]
        at org.openqa.selenium.remote.RemoteWebDriver.execute(RemoteWebDriver.java:552) ~[?:?]
        at org.openqa.selenium.remote.RemoteWebDriver.get(RemoteWebDriver.java:277) ~[?:?]
        at org.zaproxy.zap.extension.selenium.ExtensionSelenium.getProxiedBrowser(ExtensionSelenium.java:749) ~[?:?]
        at org.zaproxy.zap.extension.selenium.ExtensionSelenium.getProxiedBrowser(ExtensionSelenium.java:717) ~[?:?]
        at org.zaproxy.zap.extension.selenium.ExtensionSelenium.getProxiedBrowser(ExtensionSelenium.java:705) ~[?:?]
        at org.zaproxy.zap.extension.selenium.PopupMenuItemOpenCustomRequestInBrowser.openInBrowser(PopupMenuItemOpenCustomRequestInBrowser.java:73) ~[?:?]
        at org.zaproxy.zap.extension.selenium.PopupMenuItemOpenCustomRequestInBrowser.lambda$performAction$0(PopupMenuItemOpenCustomRequestInBrowser.java:54) ~[?:?]
        at java.lang.Thread.run(Thread.java:833) ~[?:?]

@ArkaprabhaChakraborty
Copy link
Contributor Author

Following through the redirect with the URL is what's causing the problem

@ArkaprabhaChakraborty
Copy link
Contributor Author

So when we hit a redirect the URL should be changed back to the actual URL and shouldn't stay as zapCallBack

@ArkaprabhaChakraborty
Copy link
Contributor Author

Now how da hekkk should I do this :)... webdriver.get() is what's used by the "getProxiedBrowser" :)... Can't have a selenium addition gotta do it with whatever we have :)

@ArkaprabhaChakraborty
Copy link
Contributor Author

If we have a redirect..... then kinda replace the URL in a "breakpoint-ish" fashion... that's what's coming in my mind

@kingthorin
Copy link
Member

kingthorin commented Mar 9, 2023

What happens if you re-write the location header? locationzap or locationbroken

Doesn't help for meta or js redirects, but it's a step.

@ArkaprabhaChakraborty
Copy link
Contributor Author

Rewrite? Like rewrite after I get the 302? And not follow the redirect automtically like its happening now?

@kingthorin
Copy link
Member

kingthorin commented Mar 9, 2023

I mean the user is selecting a message to open in the browser and we're feeding it from ZAP. So we know what msg is it. Just get the location header (from the response), extract the value. Set location to null, and set the value back into the message as locationzap or locationbroken. So then it's technically still part of the message but the browser shouldn't be able to redirect because it has a 3xx but no destination.

@ArkaprabhaChakraborty
Copy link
Contributor Author

I mean the user is selecting a message to open in the browser and we're feeding it from ZAP. So we know what msg is it. Just get the location header (from the response), extract the value. Set location to null, and set the value back into the message as locationzap or locationbroken. So then it's technically still part of the message but the browser shouldn't be able to redirect because it has a 3xx but no destination.

Wait ... wouldn't the browser redirect to /locationzap or /locationbroken then?

@ArkaprabhaChakraborty
Copy link
Contributor Author

So we know what msg is it. Just get the location header (from the response), extract the value.

Sometimes we will... other times it can be a direct send... like not just a replay from ZAP's stored History

@kingthorin
Copy link
Member

So if the original was location: https://demo.testfire.net/login you're going to make it locationzap: https://demo.testfire.net/login

@ArkaprabhaChakraborty
Copy link
Contributor Author

ArkaprabhaChakraborty commented Mar 9, 2023

So if the original was location: https://demo.testfire.net/login you're going to make it locationzap: https://demo.testfire.net/login

Ahhh got it :) but will it help the case? IT will stop the error but :)

@ArkaprabhaChakraborty
Copy link
Contributor Author

Actually, I don't really wanna stop the redirect... I want it to play it out... It's kinda like the "rendered" view in burp actually

@kingthorin
Copy link
Member

kingthorin commented Mar 9, 2023

Well it'll mean the redirect doesn't happen which I thought was your goal.

I guess it depends when/how it's coming up.

Is it when the original was a redirect, or is it when the original worked but "now" it's redirecting (because of an invalid session or csrf or ______)?

Edit: You already have handling detecting if it should be direct or cached when opening. This should just be an additional step shouldn't it? You should be able to figure out the conditions that lead to the situation(s) and build around them. Even if for some requests we have to refuse to open them (just show a warning dialog instead ... or "for now") 🤷‍♂️

@ArkaprabhaChakraborty
Copy link
Contributor Author

Is it when the original was a redirect, or is it when the original worked but "now" it's redirecting (because of an invalid session or csrf or ______)?

did figure that out as well ... for the test site if you login successfully you enter the "dashboard", else, you're redirected to the login page aka login.jsp

@ArkaprabhaChakraborty
Copy link
Contributor Author

Edit: You already have handling detecting if it should be direct or cached when opening. This should just be an additional step shouldn't it? You should be able to figure out the conditions that lead to the situation(s) and build around them. Even if for some requests we have to refuse to open them (just show a warning dialog instead ... or "for now") man_shrugging

TRUEEEEE BIG BRAIN MOMENT!!!! XD

@ArkaprabhaChakraborty
Copy link
Contributor Author

nope it didn't quite work

@ArkaprabhaChakraborty
Copy link
Contributor Author

So even for the "correct" login apparently there is a redirect which I missed :).... it redirects to the /bank/main.jsp with session id's and stuff

Signed-off-by: ArkaprabhaChakraborty <chakrabortyarkaprabha998@gmail.com>
Signed-off-by: ArkaprabhaChakraborty <chakrabortyarkaprabha998@gmail.com>
@psiinon
Copy link
Member

psiinon commented Aug 1, 2023

Has conflicts

@ArkaprabhaChakraborty
Copy link
Contributor Author

I think this one will be dropped. We couldn't decide a correct action for POST requests

@kingthorin
Copy link
Member

@ArkaprabhaChakraborty are you wanting to see/work this through to completion if we can settle on the POST behavior?

@ArkaprabhaChakraborty
Copy link
Contributor Author

ArkaprabhaChakraborty commented Aug 1, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants