4
\$\begingroup\$
This question is ideal in my case: Ludisposed's python-scapy-tcp-port-scan-of-subnet - At codereview stackexchange

But this is not Scapy; nor Python(it might get used in the future though but that is another question! - it is in bash - and as he(or she) stated:

Will be working on Threading later on, so that is not under review.

my case is similar.

I have been using GNU-parallel with this bash script. As well as Nmap and Hping. -Which will maybe come up as another, review question. Maybe a follow up to this one.

  • I want any thoughts - or pointers - that could improve this code (or the way I chose to do it)

  • Or if there's a better approach in general?

  • Example of anything related to that might be - Maybe I should use double quotes (at some place/(s)) instead of single quotes;

  • Use other "things" that is - maybe something else than a for loop (just as a example of what I mean)

this below is a bit of context

  • Maybe parentheses - so maybe it would make sense to (parenthesize when things go "okay" (that is &&) and vice versa (||) )
  • as it is quite simple already; it is maybe hard to simplify it even more (so this is not really under "consideration" but warmly welcomed!)

Further Usage

and some context

It should be noted I plan to extend this, with GNU-Parallel and in conjunction with Nmap - among other things. Redirect I/O (input/output), to files - read from those files and so on. Maybe find correlations between this data(data mining) (that is; multiple nodes in a network; perform analysis on all hosts that has port X open and port Y filtered - how many is Windows, how many is BSD, (...))

main concerns

  • Stability of the code, Robustness.
  • Readability(for this small code this might be funny but I'll extend this later :) and therefore want to get it right from the start (after many of my mistakes I have made previously)
  • Performance.
  • Might be unclear here but; security. I hope I am following not only the guidelines and all good-practice - but also writing securely.

the code

#!/usr/bin/env bash
## shebang from the below sources
# https://stackoverflow.com/a/10383546/14346786
# https://stackoverflow.com/questions/10376206/what-is-the-preferred-bash-shebang

export A=0  # start of the port range
export B=53 # end   of the port range

for p in $(seq $A $B); do
    timeout 1s bash -c '</dev/tcp/192.168.1.1/'$p'' && echo "[+] $p open" || echo "[-] $p closed/filtered/timed out"
done

I struggled a lot previously ( awhile ago I confused sh with bash and -c "" (..) and confused quotes; && - and so on) but now it is working; which is why I went here - And really (hopefully successfully)

really simplified it into 1 or 2 lines. (to make it so easy to read as possible)

I have tried (and plan on using in the future):

  • C program
  • Python(mostly v.3)
  • Nmap (to so-called enumerate the (supposedly open) ports (what services, what versions of these services, ...)

(excluding extended versions of this where - maybe if a port(80 or HTTPS) is open; I will use the GET command to grab the HTML from the remote host) or, a similar case - where waiting is included)

I should've mentioned That I searched this site for dev tcp (and bash) but it came few results,

\$\endgroup\$
0

2 Answers 2

4
\$\begingroup\$

first shellcheck and quoting

Shellcheck is a good first place to start and you did very well here. The only complaint that shellcheck has is around the quoting of variables in the timeout line. There is a chance that $p could be empty or contain spaces. If $p contained spaces and you're interpolating it without quotes then the shell would take it as two arguments. This is usually not desired and will cause bugs. So shellcheck wants you to quote it.

Quoting $p is a bit of a challenge since you're already surrounding it with quoted things. Take '</dev/tcp/192.168.1.1/'$p'': the variable is unquoted but surrounded by two single quoted strings. As you may have discovered you can't interpolate variables in single quoted strings. Or you can, as you have here, and it makes things a bit messy-looking to me. There are also double quoted strings in the shell. Double quoted strings are handy because they allow interpolation inside of them but other special characters are disabled. So your '</dev/tcp/192.168.1.1/'$p'' becomes "</dev/tcp/192.168.1.1/$p"

inside the loop

Your line inside of the loop is nice if you were trying to run this on the command line, but it will be harder to expand and tweak. You can put this in an if block. The timeout command becomes the conditional. You don't need square brackets. The return value of the command is what the if looks at. Just if timeout... then put your commands in the true/false sections of the if/else/fi.

Once you split up the one-liner I'd also rename p to port to make it clear what it is.

good points

  • nice indentation
  • yay for the shebang
  • using seq in a sensible way
  • nice looking output
\$\endgroup\$
2
  • \$\begingroup\$ Thanks soo much for the feedback and I was afraid of something really crazy wrong; but thanks! will look into every word you wrote!and... Thanks a Lot for the good points! especially the yay for the shebang.. :D made my day! \$\endgroup\$ Feb 9 at 7:03
  • \$\begingroup\$ will come back when I am done; I can be quite slow some times but - really - appreciate it! \$\endgroup\$ Feb 9 at 7:18
2
\$\begingroup\$

I don't see a need to execute this script using Bash. Plain POSIX sh should be fine in the shebang.

The only Bash feature used is /dev/tcp/…; that's within the bash -c '…' embedded program, and can remain as-is.


It's a good idea to use set -u so that unset variables cause a script error. That helps avoid simple mistakes. I also recommend set -e, so that scripts don't continue after a failed command, except where we make use of the failure status.


There's no need to put A and B in the environment. Just set them as ordinary shell variables, without export. It's always a good practice to use lower-case for variables, as upper-case is generally used for environment variables which change programs' behaviour (e.g. LANG, TZ, TERM).


I recommend redirecting the error stream from the bash invocation, as we will be reporting the status directly.


Instead of && and ||, it's better (and clearer) to use if and else in this program.


Modified code

#!/usr/bin/env sh

set -eu

# Port range
start=0
end=53

for p in $(seq "$start" "$end")
do
    if timeout 1s bash -c "</dev/tcp/192.168.1.1/$p" 2>/dev/null
    then echo "[+] $p open"
    else echo "[-] $p closed/filtered/timed out"
    fi
done
\$\endgroup\$
4
  • \$\begingroup\$ @TonySpeight thanks a lot; will check this, I greatly appreciate all this help; \$\endgroup\$ Feb 9 at 8:39
  • \$\begingroup\$ @TonySpeight If I would use multiple commands after say - a port is open; I would connect to it (via NC or something else) would it make sense here to use parentheses or should I just put the code (to be executed if a port is open) "inside" the then statement? Note: thanks (again) so much for the help; both answers are terrific - I wish I was able to accept 2 answers; \$\endgroup\$ Feb 9 at 16:42
  • 1
    \$\begingroup\$ Only use parentheses ((…)) if you need a subshell. That's their function in Bourne-family shells. if/else/then/fi provide their own grouping, so no need for braces ({…}) either. If it helps, use extra line breaks (as I've shown following do) to make the structure clearer with indentation. \$\endgroup\$ Feb 9 at 17:07
  • \$\begingroup\$ oh okay; that cleared it up very well; was maybe a trivial question but;really thanks! \$\endgroup\$ Feb 9 at 17:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.