0
\$\begingroup\$

I don't know how to use classes and i think its meant to be used here to avoid repetition of functions but i can be completely wrong, i am a student who keeps learning bit by bit. Please can you take a look into my code? i truly appreciate it :)

import nmap
import re
from sys import argv, version
#Open the file then read the content and print it
f= open("texttop.txt", "r")
file_contents= f.read()
print(file_contents)
sperate= "--------------------------------------------------------------------------------------------------------"
#shorten the function with a variable
scanner = nmap.PortScanner()
#Get the target ip address and the port range from the terminal
args = argv[1:]
if len(argv)==4:
    target_ip=args[0]
    port_range=args[1]
    scan_type=args[2]
else:
    print("Your supposed to enter the Target IP address then the port range and the attack type, enter them again.")
    target_ip= str(input("Enter the target IP address:\n"))
    port_range= str(input("Enter the port range please:\n "))
    scan_type=str(input("Enter attack type"))
    port_range.lower()   
#Function for scan 1 
def start_scan1():
    #print the nmap version to the user
    nversion = "NMAP.Version:" + str(scanner.nmap_version())
    #Print nmap version without any special characters
    string = re.sub(r"[() ]","",nversion)
    #start scanning
    print(string+"\nScanning...")
    scanner.scan(target_ip,port_range,"-v -sA -O -sV")
    #Save the scan info into a variable
    print("This a list of the scan info. Please ignore if not wanted: " + str(scanner.scaninfo()))
    #For every host in the target aquired we check its status
    for host in scanner.all_hosts():
        print(sperate)
        #print the status of the target
        print('Host : %s (%s) \t State : %s \n ' % (host, scanner[host].hostname(),scanner[host].state()))
        #Check the protocol for every host
        for proto in scanner[host].all_protocols():
            print("Protocol: %s" % (proto))
            #Make a variable to get the ports from the protocol
            lport = scanner[host][proto].keys()
            #Check port status
            for port in lport:
                print("Port: %s \t State: %s \t Service: %s \t Version: %s" % (port, scanner[host][proto][port]['state'],scanner[host][proto][port]['name'],scanner[host][proto][port]["version"]))
            print("--------------------------------------------------------------------------------------------------------")
            print("\n Finished scan.")
#Function for scan 2
def start_scan2():
    #print the nmap version to the user
    nversion = "NMAP.Version:" + str(scanner.nmap_version())
    #Print nmap version without any special characters
    string = re.sub(r"[() ]","",nversion)
    #start scanning
    print(string+"\nScanning...")
    scanner.scan(target_ip,port_range,"-sS -T0")  
    #Save the scan info into a variable
    print("This a list of the scan info. Please ignore if not wanted: " + str(scanner.scaninfo()))
    #For every host in the target aquired we check its status
    for host in scanner.all_hosts():
        print(sperate)
        #print the status of the target
        print('Host : %s (%s) \t State : %s \n ' % (host, scanner[host].hostname(),scanner[host].state()))
        #Check the protocol for every host
        for proto in scanner[host].all_protocols():
            print("Protocol: %s" % (proto))
            #Make a variable to get the ports from the protocol
            lport = scanner[host][proto].keys()
            #Check port status
            for port in lport:
                print("Port: %s \t State: %s \t Service: %s \t Version: %s" % (port, scanner[host][proto][port]['state'],scanner[host][proto][port]['name'],scanner[host][proto][port]["version"]))
            print("--------------------------------------------------------------------------------------------------------")
            print("\n Finished scan.")
#if statement to check which scan combination the user want to use.
if scan_type==1:
    start_scan1()
elif scan_type==2:
    start_scan2()
else:
    start_scan1()
\$\endgroup\$
4
\$\begingroup\$

This is by no means a thorough review but I see things you can improve in no particular order:

  • add line spacing, especially between functions - that will make the code easier to read. Everything is glued together here, it's not obvious in the blink of an eye where a function start and where it ends
  • for command line arguments, you should be using the argparse module that is less clumsy and allows you to accept named parameters without a preset order. A short example here
  • it's not clear what file you are reading on top of your code. If it is a bunch of instructions then maybe it would be more elegant to put all that in a separate .py file and import it. If it is help text, then read above about argparse.
  • you are not validating the parameters, so your program can easily misbehave in case of typos, the obvious way would be to use regular expressions to validate IP address etc. You are already using the re module for something else.
  • the validation can be baked in with argparse, here's another more sophisticated example (quoting myself again)
  • unnecessary repetition: start_scan1 and start_scan2 are obviously nearly identical. The only thing that that is different is the scan parameters. So you could just create a single function with scan options as arguments. Just doing that will decrease your code base by 30%.
  • but these functions are poorly named, the purpose should be intuitive from the function name so they could be named scan_paranoid and scan_aggressive respectively. You can write simple, one-liner functions with these descriptive names, that will call a more generic scan routine with the desired parameters.
  • you should use the logging module to send output to both console and file. A basic example here. The reason is obvious: it's easier to review results in a file, especially if the results are properly formatted (you could even output as CSV). The other reason is that the console buffer size is limited and you may lose output after spitting out a few hundreds or thousands of rows, which would occur in a large scan.
  • instead of %s formatting, you could use F-strings (requires Python 3.6). If you are on a recent version of Python3 I would recommend you to upgrade the coding style in this regard

Code

At line #22:

port_range.lower()

This does nothing, this is not a variable assignment

sperate is a typo. Besides, it could be used on line #47, rather than repeating those hyphens. It's no hugely useful anyway.

Coding style

Review PEP8 for coding style, for example there should be a space after every comma between function arguments and that convention should be consistently respected throughout your code, which is not the case here:

    print('Host : %s (%s) \t State : %s \n ' % (host, scanner[host].hostname(),scanner[host].state()))

A good IDE with linter (eg flake8) can definitely pinpoint the problematic patterns and help enforce good habits.

Some lines are way too long (eg #46, 72). They should be broken down to multiline declaration for better readibility. Your screen is quite likely larger than mine and you may not have to scroll. But chunks of code overly large should be avoided nonetheless - they are not pleasant to review. PEP8 actually recommends 79 characters width - I don't respect this rule strictly but remain reasonable.

Misc

I am quite familiar with nmap but never used the corresponding Python module. You might also be interested in Scapy, that does a different job actually, in the sense that it does not interprets results for you, but returns raw packet responses instead. It may be interesting if one day you need to dig deeper into the responses than nmap allows.

\$\endgroup\$
1
  • \$\begingroup\$ Truly this is quite useful and i appreciate every bit of information you handed me, i chose nmap rather than Scapy because i am not quite qualified for it yet, its a bit hard but i appreciate your help again i truly do. \$\endgroup\$
    – BackpainYT
    Aug 10 at 17:05

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.