-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
feat: Added time based delay analyzer to fuzzing implementation #5781
Conversation
… fuzzing-additions
… analyzers-fuzzing-additions
By design, users won’t really know how the Another key point to ensure is that the output type is boolean. So, instead of doing it like this: matchers:
- type: word
# Also, DSL part analyzer_details contains detailed analysis
# from the time delay analyzer.
part: analyzer_matched
words:
- "true" We could go with a more straightforward approach by defining the matcher as: matchers:
- type: dsl
dsl:
- time_delay # shorthand: time_delay == true This is clearer and easier to follow. |
Is Here's what I'm thinking: analyzer:
- type: time_delay
name: delayed # exported
parameters: {...}
- type: another_analyzer # what if there's no `name`? Defining `analyzer_N`?
parameters: {...}
matchers:
- type: dsl
dsl:
- delayed == true |
@dwisiswant0 Regarding the first point of matchers:
- type: dsl
dsl:
- analyzer_matched == true Regarding the second point, only one analyzer is supposed to be used per check. For instance, the user won't combine time and boolean or xss context together. So it doesn't make sense to allow user to configure multiple of them. I don't see a scenario where we might need to combine multiple. Let me know what you think! |
pkg/fuzz/analyzers/time/analyzer.go
Outdated
// Default unit is second. If we get passed milliseconds, multiply | ||
if unit, ok := params["time_unit"]; ok { | ||
duration = a.handleCustomTimeUnit(unit.(string), duration) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be more reliable to use time.ParseDuration
directly and skip having a custom handler for it? So we could remove the "time_unit" param entirely and make "sleep_duration" a string that gets parsed into time.Duration
. What do you think, @Ice3man543?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, will make it like so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left some comments, but overall, this is a strong PR that will enable the addition of many valuable analyzers.
// Name returns the name of the analyzer | ||
Name() string | ||
// ApplyTransformation applies the transformation to the initial payload. | ||
ApplyInitialTransformation(data string, params map[string]interface{}) string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should separate this from the Analyzer, as the Analyzer is not a Transformer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, we can separate Analyzer to another file to keep the code more clean.
// Analyzer is a time delay analyzer for the fuzzer | ||
type Analyzer struct{} | ||
|
||
const ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can add comments to explain why these are the default values --it's not obvious at first glance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Ice3man543 can we add docs at https://github.com/projectdiscovery/docs as analyzer section with details of time_delay
Proposed changes
Added time delay verification by using alternating requests logic from ZAP to nuclei DAST mode and also added the concept of analyzers allowing more complicated verification checks. (HTTP - Fuzzing only)
Example template:
Checklist