21y4d / nmapAutomator

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

Code cleanup + some bug fixes #42

Closed caribpa closed 3 years ago

caribpa commented 3 years ago

Refactor the code so that the variables are easily identifiable + some robustness improvements (the variables should be quoted) and general clutter cleanup.

This time I've tested the script (last time I was checking the wrong branch) 🙂

21y4d commented 3 years ago

Wow! Another amazing effort. Again, i'll review the changes and test them and will then merge the pull request.

Can we say that the script is now more reliable on various types sh of shells?

caribpa commented 3 years ago

Can we say that the script is now more reliable on various types sh of shells?

As of now the script is bash specific unless you call it with other shells like: zsh nmapAutomator.sh or sh nmapAutomator.sh (though sh may be bash itself as it is system specific).

This said, the script is now more robust (meaning: less chances to break under unexpected input/grep-parsing, etc).

But don't worry as I plan to make this script 100% POSIX so that it can be executed in any shell (sh, bash, zsh, ash, dash, etc). Reaching POSIXness is important as it will allow to run the script in any machine with a POSIX shell, really useful for the PWK course and OSCP exam as you would be able to run the script in any victim's machine (providing they have nmap installed, though sending a static nmap to the victim is always possible) targeting the internal network (hugely increasing the speed of the nmap scan).

21y4d commented 3 years ago

Excellent. This should really increase the use cases and productivity in general of the script. Thanks again for the amazing efforts.

caribpa commented 3 years ago

What I think it is important is to have a "coding standard" document (or section in the README), basically saying to:

21y4d commented 3 years ago

Yeah definitely, to ensure it doesn't break POSIX and readability. I'll add that either to README or its own document.

I think being 100% POSIX would be huge, especially for lateral movement and internal network scanning and enumeration. In this case i may add a flag for nmap path, that defaults to the default path if not specified. This is to enable using a static nmap when needed.

I'll also wait on testing and reviewing the changes if you have any more changes coming, so that i don't have to go through them twice :)

caribpa commented 3 years ago

I'll create a different pull request for the POSIX changes, you can wait if you prefer though I will only have time to start working on them in around 6 hours 🙂

11nf0s3c commented 3 years ago

Great effort @caribpa. Well done.

21y4d commented 3 years ago

I think once you're done i'll work on adding a --remote mode, where the script switches to using POSIX commands only, even for basic recon.. i'll also add --nmapDir to allow for transferring nmap for --remote mode.

This'll make nmapAutomator great for lateral movement.

caribpa commented 3 years ago

Amazing idea! 👍

Btw, you can merge these changes as I am working on the POSIX refactor at this branch and I don't think I'll finish today.

This said, I've found some other things that could use some cleanup, though I won't work on them until the POSIX changes are finished 🙂

21y4d commented 3 years ago

Thanks.. I usually work on nmapAutomator during the weekend, so you can take as much time as you'd like :)

I'll try to work on merging the changes as soon as I can, though.