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

Fix progress with patterns and backup discovery #548

Open
wants to merge 7 commits into
base: dev
Choose a base branch
from

Conversation

acammack
Copy link

@acammack acammack commented Jan 29, 2025

Hi all, this resolves the issues where patterns were not counted towards possible attempts, file extension searching was not applied to patterns in GobusterDir, and backups were being brute-forced for the entire wordlist. These all had pretty tangled fixes so I lumped them into this PR.

$  ./gobuster dir -p patterns.txt -w testwords.txt -x .php,.html -db -u https://testing.test
===============================================================
Gobuster v3.7
by OJ Reeves (@TheColonial) & Christian Mehlmauer (@firefart)
===============================================================
[+] Url:                     https://testing.test
[+] Method:                  GET
[+] Threads:                 10
[+] Wordlist:                testwords.txt
[+] Patterns:                patterns.txt (2 entries)
[+] Negative Status codes:   404
[+] User Agent:              gobuster/3.7
[+] Extensions:              php,html
[+] Timeout:                 10s
===============================================================
Starting gobuster in directory enumeration mode
===============================================================
/index.html           (Status: 200) [Size: 179]
/index.html.bak       (Status: 200) [Size: 179]
Progress: 33 / 33 (100.00%)
===============================================================
Finished
===============================================================
$ wc -l testwords.txt patterns.txt 
 3 testwords.txt
 3 patterns.txt
 5 total

Note the 33 total attempts for:

3 words * 3 patterns * (2 file extensions + no file extension) = 27 original attempts
plus 6 backup styles for the original find of "index.html"

Since it's in this wheelhouse, I'm open to also changing the pattern generation and wordlist behavior to either have a flag for the exclusion of the bare words from the wordlist that's either on or off by default or even make patterns always supersede the wordlist and make users add a plain {GOBUSTER} pattern if they want the original word in addition to the patterns. I'm not sure which would be the most useful, though.

Looking through the old issues, it seems that (1) filtering the whole wordlist through the patterns and (2) applying the discovery patterns exactly once would be the most consistent behavior.

Fixes #298, #405, #480, #533, #319, #364

There are still a couple of things to do before this is ready, but I wanted to get this open for some eyes:

  • I need to track successes from success pattern generation to keep it from infinitely generating requests against malicious services/with misconfiguration, possibly with a configurable chain generation limit
  • I want to add a global flag for post-find pattern expansion, so that if the user desired everything could be checked with patterns after a successful find

Specifying patterns for the word list will no longer cause progress to
go past 100%. Additionally, the GobusterDir transformations for file
extensions and backups will be applied after pattern expansion.

Fixes OJ#405, OJ#480, and OJ#533
This is done by re-arranging the code from exiting on channel close to
using the contexts and the results counters to signal the end of work
processing. A little more work is needed to prevent infinite loops
caused by devious services/misconfiguration and to expose to the cli the
ability to have patterns run on successful finds.

Fixes OJ#298
@firefart
Copy link
Collaborator

Nice thanks already! One thing I noticed is that we should close all channels when we are done sending on them

@acammack
Copy link
Author

One thing I noticed is that we should close all channels when we are done sending on them

Hmm, is that necessary here? By the time we would close the wordChan and moreWordsChan channels, all the other goroutines referring to them will have finished and close won't have anyone to notify. The resources for the channels still won't be freed up until after Run() returns either way. I guess it wouldn't hurt anything either, but I feel it would muddy the intent of the code by implying that there might be consumers that need to be notified that the channels are done.

@firefart
Copy link
Collaborator

ok, so as long as no one is waiting for a close, it should be ok then

@acammack
Copy link
Author

Yes, all the producers and consumers are terminated by contexts now and verified with wait groups and request counts. I think this also fixes a few minor issues that have already been closed, like #541

@firefart
Copy link
Collaborator

Awesome, so just put the PR from draft to normal once you are ready so I can have a test. Thanks so far!

From the Go spec:
> If one or more of the communications can proceed, a single one that
> can proceed is chosen via a uniform pseudo-random selection.

Previously, this meant that some indeterminate amount of work could have
been completed after the context's cancel function had been called.
Successful guesses from the wordlist or a pattern will have discovery
patterns generated based on them and successful discovery guesses will
not. Further processing should require human curation to avoid
automatically generating an unbounded amount of traffic.

Also fixes reading the wordlist from standard in by making it more like
reading from a file now that we have dynamic progress updates.
@acammack acammack marked this pull request as ready for review February 5, 2025 05:39
@acammack
Copy link
Author

acammack commented Feb 5, 2025

This should be good to test. I think the changes in here will help with adding machine readable results and a pause/state file, though I'll leave those to the future. I think we could also make our string manipulation more efficient, but that's also another PR.

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

Successfully merging this pull request may close these issues.

2 participants