paramdigger: Add patches for header guesser errors #4561
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Needs cleanup. Comments and System.out 😉 |
thc202
changed the title
|
The changelog should be updated. |
kingthorin
reviewed
Apr 25, 2023
addOns/paramdigger/src/test/java/org/zaproxy/addon/paramdigger/HeaderGuessUnitTest.java
Outdated
Show resolved
Hide resolved
|
I would actually keep it... Just so that I can comeback and review the
logic :)
Its in tests tho... Shouldn't really affect but :)...
…On Wed, Apr 26, 2023, 2:49 AM Rick M ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In
addOns/paramdigger/src/test/java/org/zaproxy/addon/paramdigger/HeaderGuessUnitTest.java
<#4561 (comment)>
:
> @@ -236,7 +244,7 @@ protected Response serve(IHTTPSession session) {
response.addHeader(header, value);
response.addHeader(
"host",
- ((poisoning.get("host")) != null) // && value == "hit"
+ ((poisoning.get("host")) != null) // && value == "hit")
Remove the commented bit?
—
Reply to this email directly, view it on GitHub
<#4561 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AN6F3GLXTNW4EJCQI3V5FZ3XDA5UTANCNFSM6AAAAAAXIXYJXQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
*would actually like
On Wed, Apr 26, 2023, 3:03 AM Arkaprabha Chakraborty <
***@***.***> wrote:
… I would actually keep it... Just so that I can comeback and review the
logic :)
Its in tests tho... Shouldn't really affect but :)...
On Wed, Apr 26, 2023, 2:49 AM Rick M ***@***.***> wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In
> addOns/paramdigger/src/test/java/org/zaproxy/addon/paramdigger/HeaderGuessUnitTest.java
> <#4561 (comment)>
> :
>
> > @@ -236,7 +244,7 @@ protected Response serve(IHTTPSession session) {
> response.addHeader(header, value);
> response.addHeader(
> "host",
> - ((poisoning.get("host")) != null) // && value == "hit"
> + ((poisoning.get("host")) != null) // && value == "hit")
>
> Remove the commented bit?
>
> —
> Reply to this email directly, view it on GitHub
> <#4561 (review)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AN6F3GLXTNW4EJCQI3V5FZ3XDA5UTANCNFSM6AAAAAAXIXYJXQ>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
|
|
Add TODO comments or tasks in the issue instead. |
thc202
reviewed
May 1, 2023
thc202
reviewed
May 1, 2023
addOns/paramdigger/src/main/java/org/zaproxy/addon/paramdigger/CacheController.java
Outdated
Show resolved
Hide resolved
addOns/paramdigger/src/main/java/org/zaproxy/addon/paramdigger/HeaderGuesser.java
Outdated
Show resolved
Hide resolved
addOns/paramdigger/src/main/java/org/zaproxy/addon/paramdigger/HeaderGuesser.java
Outdated
Show resolved
Hide resolved
addOns/paramdigger/src/main/java/org/zaproxy/addon/paramdigger/HeaderGuesser.java
Outdated
Show resolved
Hide resolved
addOns/paramdigger/src/main/java/org/zaproxy/addon/paramdigger/HeaderGuesser.java
Outdated
Show resolved
Hide resolved
addOns/paramdigger/src/main/java/org/zaproxy/addon/paramdigger/HeaderGuesser.java
Outdated
Show resolved
Hide resolved
ArkaprabhaChakraborty
force-pushed
the
arka/header-guesser-patch
branch
from
088054b
to
d17172b
Compare
kingthorin
reviewed
May 1, 2023
addOns/paramdigger/src/test/java/org/zaproxy/addon/paramdigger/HeaderGuessUnitTest.java
Outdated
Show resolved
Hide resolved
|
uhh yeah about that :)
…On Mon, May 1, 2023, 7:49 PM Rick M ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In
addOns/paramdigger/src/test/java/org/zaproxy/addon/paramdigger/HeaderGuessUnitTest.java
<#4561 (comment)>
:
> + // String retVal = session.getHeaders().get("host");
+ // if (retVal.contains(":31337")) {
+ poisoning.put("host", session.getHeaders().get("host"));
+ // }
😉
—
Reply to this email directly, view it on GitHub
<#4561 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AN6F3GKNQQ5BGSBNJTWLJTLXD7BA5ANCNFSM6AAAAAAXIXYJXQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
ArkaprabhaChakraborty
force-pushed
the
arka/header-guesser-patch
branch
from
d17172b
to
683f98a
Compare
Signed-off-by: ArkaprabhaChakraborty <chakrabortyarkaprabha998@gmail.com>
ArkaprabhaChakraborty
force-pushed
the
arka/header-guesser-patch
branch
from
683f98a
to
a13ad2d
Compare
|
That should be it! :) |
|
ping @thc202 |
thc202
approved these changes
May 15, 2023
Comment on lines
+254
to
+264
| switch (method) { | ||
| case GET: | ||
| headers.setMethod(HttpRequestHeader.GET); | ||
| break; | ||
| case POST: | ||
| headers.setMethod(HttpRequestHeader.POST); | ||
| break; | ||
| default: | ||
| throw new IllegalArgumentException( | ||
| "Method " + method + " not supported."); | ||
| } |
There was a problem hiding this comment.
Does not need to be in this PR, letting the enum update the request header would reduce duplication and simplify the code.
| || !this.checkCacheHit(indicValue, cache)); | ||
| if (indicValue == null || indicValue.isEmpty()) { | ||
| return true; | ||
| } else { |
There was a problem hiding this comment.
These else statements could be removed.
thc202
reviewed
May 15, 2023
| @@ -74,6 +74,7 @@ public class HeaderGuesser implements Runnable { | |||
| private static final String POISON_DEFINITION = "paramdigger.results.poison.definition"; | |||
| private static final String POISON_DEFINITION_FIRST = | |||
| "paramdigger.results.poison.definition.first"; | |||
| private static List<Integer> ERROR_CODES = List.of(400, 413, 418, 429, 503); | |||
|
regarding these else statements..... I thought of adding an
error/debug/output panel which vividly describes what's wrong or what
happened :).... But it also gave me an idea that we can have an entire
logging panel for ZAP :).......
…On Mon, May 15, 2023, 12:10 PM thc202 ***@***.***> wrote:
***@***.**** approved this pull request.
------------------------------
In
addOns/paramdigger/src/main/java/org/zaproxy/addon/paramdigger/CacheController.java
<#4561 (comment)>
:
> + switch (method) {
+ case GET:
+ headers.setMethod(HttpRequestHeader.GET);
+ break;
+ case POST:
+ headers.setMethod(HttpRequestHeader.POST);
+ break;
+ default:
+ throw new IllegalArgumentException(
+ "Method " + method + " not supported.");
+ }
Does not need to be in this PR, letting the enum update the request header
would reduce duplication and simplify the code.
------------------------------
In
addOns/paramdigger/src/main/java/org/zaproxy/addon/paramdigger/CacheController.java
<#4561 (comment)>
:
> @@ -829,9 +859,22 @@ private boolean checkAlwaysMiss(String url, Method method, Cache cache) {
// faced.
}
String indicValue = msg.getResponseHeader().getHeader(cache.getIndicator());
- return (indicValue == null
- || indicValue.isEmpty()
- || !this.checkCacheHit(indicValue, cache));
+ if (indicValue == null || indicValue.isEmpty()) {
+ return true;
+ } else {
These else statements could be removed.
—
Reply to this email directly, view it on GitHub
<#4561 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AN6F3GMOHSJ6BRBQ3KJGI6DXGHFUFANCNFSM6AAAAAAXIXYJXQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
That's what the Output panel is (IMHO) |
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related to: zaproxy/zaproxy#7694