21y4d / nmapAutomator

A script that you can run in the background!
MIT License
2.67k stars 790 forks source link

Convert to POSIX + bug fixes #43

Closed caribpa closed 3 years ago

caribpa commented 3 years ago

These commits make the script 100% POSIX (tested in dash) except for the tools that cannot be converted, such as nmap, ping, and the scan/vuln tools (nikto, ffuf, etc).

The changes from #42 are also here (as I thought they would have been merged before I finished), so I recommend to merge first the other pull request and then check the changes here.

It shall be noted that the changes from #42 introduced a little bug (in a Full Scan the same ports used in the Quick Scan where re-detected), as well as showing (in some cases when running as root) closed ports. These two bugs are fixed in this pull request, though nmap is now run with the --open flag (most recent commit) to only show open ports (in the commit message I explained more, and also suggested adding a new flag to nmapAutomator called --show-all-ports that basically runs nmap without the --open flag).

Finally, this pull request fixes the frozen progress bar (Full Scan) when running the script as root 🙂

21y4d commented 3 years ago

Wow.. thanks once again for the massive efforts on making it 100% POSIX. This should really help on making the script work on remote environments.

Please allow me some time to go through all changes and properly test them, to ensure nothing breaks and everything works as expected.

Thanks

caribpa commented 3 years ago

It would be a good idea to check whether nmap and ping exist (nikto and the others are checked, though the script expects to find them in a bin folder, this should be changed as well).

One other thing that would be good to have is the ability to disable the colors (--no-color flag) as there may be some dumb/broken terminals/shells/tty/pty that don't recognize the ANSI escapes. Ideally, the script should be able to detect if colors are supported (like at least GNU grep does when writing to stdout vs a pipe).

Once these changes are merged I'll continue refactoring 🙂

caribpa commented 3 years ago

This should really help on making the script work on remote environments.

Btw, these changes guarantee portability, you will be able to run the script in your 10yo cheapo ISP router through a reverse shell spawned via telnet as long as you can cross-compile a static nmap compatible with it 😃

21y4d commented 3 years ago

I have done quick testing, and everything seems ok.. I only noticed one issue: if no recon is found, it would still show the timer. I guess this is probably because $availableRecon contains a comma or something in all cases, so we should count for that.

Btw i'm really impressed with how you turned cmpPorts() to a single line! That's some neat thinking :) The changes in general are very clean and impressive as well, very nice work.

I'll try to do full testing tomorrow, and then I'll merge it if everything runs ok. Then i can start working on the remote/network scanning features as well.

caribpa commented 3 years ago

I only noticed one issue: if no recon is found, it would still show the timer. I guess this is probably because $availableRecon contains a comma or something in all cases, so we should count for that.

Probably https://github.com/21y4d/nmapAutomator/pull/43/commits/4f4fe679e13491c194fd2eed7fcd95895875c1a6 fixes it, can you confirm if it does?

caribpa commented 3 years ago

Btw i'm really impressed with how you turned cmpPorts() to a single line! That's some neat thinking :)

Usually, I have to control myself to avoid one-lining everything, glad you like it 😂

The changes in general are very clean and impressive as well, very nice work.

👍 I have a list with more refactoring I would like to do after these changes are merged 🙂

caribpa commented 3 years ago

I have done quick testing, and everything seems ok.

Btw, I'd like to know what you think regarding the nmap --open flag (added https://github.com/21y4d/nmapAutomator/pull/43/commits/e9d99f03b02d9fe411ca6a2978e9c20c5cd16b1d) as this is the "biggest" change in functionality as the nmap scan presented to the user will only show the open ports. This means that the filtered ports (showed along the open ports to the user in the current version) are no longer presented to the user in the scan reports.

Personally, I don't pay attention to the filtered ports (and manually run nmap with the --open flag), but I would like to know your stance in this.

Also, according to this answer, using the -A, or the -sV (the latter is currently only used in Vulns scan) can help in identifying the real status of the port. Maybe, can they be considered?

21y4d commented 3 years ago

nmapAutomator always only considers open ports. When i first developed the script i intentionally did not use the --open flag, but i can't remember exactly why. Perhaps it had to do with output or speed. I guess if everything runs as expected, we should be fine using it.

The -sV only runs service/version detection, which is what we need. The -A flag runs more unnecessary things like traceroutes and such, which slows the scans for no proper reason. I always preferred adding more flags to the nmap commands to make it more curated, rather than running everything, to make it as fast and efficient as possible. This was a couple of years ago, so things may have changed now.

caribpa commented 3 years ago

The -sV only runs service/version detection, which is what we need. The -A flag runs more unnecessary things like traceroutes and such, which slows the scans for no proper reason.

Yes, I agree. Anyways, maybe -sV should be benchmarked as it may resolve almost-for-free the conflicts of false filtered ports (due to unlucky packet drops) and open|filtered ones (because I'm not sure how the script reacts to these ones).

21y4d commented 3 years ago

I've merged both pull requests after i did some proper testing.

I noticed and fixed a couple of issues with the recon, as follows:

For the first issue, i simply removed the extra comma. For the second, i replaced spaces with newlines, so that the awk command would properly remove missing tools and keep the rest.

You can check the latest commit, and see if these changes would conflict with any of your changes.