Implementation of portscanner for a host

Question

I have tried to check whether the ports of a host from port 0 to port 1023 is open or not. Could someone please review this code and provide feedback.

#include<stdio.h>
#include<stdlib.h>
#include<string.h>
#include<sys/socket.h>
#include<sys/types.h>
#include<netdb.h>
#include<error.h>
#include<errno.h>
char *convert_int_to_string(int num);

char *convert_int_to_string(int num)
{
    int tmp;
    int i = 0;
    int j = 0;
    static char a[5] = {'0'};

    while (num > 0) {
        tmp = num % 10;
        a[i++] = tmp + '0';
        num = num / 10;
    }
    a[i] = '\0';
    for (j = 0; j < i / 2; j++) {
        tmp = a[j];
        a[j] = a[i - j - 1];
        a[i - j - 1] = tmp;
    }
    return a;
}


int main(int argc, char **argv)
{
    int status;
    char *node;
    char *port_no;
    int sock_fd;
    int i = 0;
    struct  addrinfo hints, *serviceinfo;

    if (argc != 2)
        error(1, errno, "Too many or few arguments\n");
    node = argv[1];
    memset(&hints, 0, sizeof hints);
    hints.ai_family = AF_INET;
    hints.ai_socktype = SOCK_STREAM;
    for (i = 0; i < 1024; i++) {
        port_no = convert_int_to_string(i);
        status = getaddrinfo(node, port_no, &hints, &serviceinfo);
        if (status != 0) {
            error(1, errno, "error in getaddrinfo() function call\n");
        }
        sock_fd = socket(serviceinfo->ai_family, serviceinfo->ai_socktype, serviceinfo->ai_protocol);
        if (sock_fd == -1)
            error(1, errno, "error in socket() function call\n");
        status = connect(sock_fd, serviceinfo->ai_addr, serviceinfo->ai_addrlen);
        if (status != -1)
            printf("Port : %s is open\n", port_no);
        else
            printf("Port : %s is closed\n", port_no);
    }
}

Answer 1

Here are some things that may help you improve your program.

Use only required #includes

The code uses nothing from <stdlib.h>, so that header can and should be omitted. Only include files that are actually needed. This makes the code easier to understand and maintain and also may slightly speed up compiling.

Try to write portable code

Things that are guaranteed by the C standard are absolutely portable to conforming compilers on all platforms. Things that are specified by POSIX standards are portable to the subset of machines that conform to POSIX. Your life will be easier if you aim for those. Things that are compiler extensions, such as error are not necessarily portable to other machines. For that reason, I'd suggest that instead of using error in this program (which is a GNU extension), you could use printf and return or exit. For example, instead of this:

if (argc != 2)
    error(1, errno, "Too many or few arguments\n");

Write this:

if (argc != 2) {
    printf("Incorrect number of arguments.\n"
            "Usage: %s node\n"
            "where node is the IPv4 address of a machine to be scanned.\n", argv[0]);
    return 1;
}

Think about the user

In the code above giving the user an error message saying "Too many or few arguments" is likely to be a frustrating experience for the user. You say what's wrong from the program's point of view, but don't say what to do differently from the user's point of view. The suggested text above does both.

Use standard library functions

The convert_int_to_string has a bug (it returns a null string if the passed number is zero) and isn't really needed anyway. You could either use snprintf or write a function to increment a text version of the port.

Understand library calls

The getaddrinfo call returns a linked list in serviceinfo but this program never frees that memory. You should instead call freeaddrinfo(serviceinfo); and both should be outside the loop. You may also consider trying each of the items in that linked list; it's common, for instance that localhost resolves to both ::1 and 127.0.0.1 and also common for ports to be open on only IPv6 or only IPv4.

Don't use up resources

The number of simultaneously open file handles is typically set per system. If you're running Linux, you can use the ulimit -n command to find out how many are available on your system. A typical value is 1024, but remember that stdin, stdout and stderr are 3 of those. Any other open files also subtract from that. This means that it's easy to run out of them unless you close handles as soon as you are finished with them. In this case, it means that every call to socket must be paired with a corresponding close(sock_fd); or you are likely to run out of handles. Generally: free memory you allocate and close file descriptors you open.

Don't forget about IPv6

There doesn't seem to be a good reason to restrict this code to IPv4 only when it could work just as well with IPv6. To accommodate either, simply delete the line that sets hints.ai_family.

Understand real world consequences

Running a port scan should only be done on computers that are yours or on computers for which you have permission to do so. Smart security professionals get such permission in writing before commencing. See this SANS whitepaper for more information on this topic.

An example

One way to apply these suggestions could look like this:

int main(int argc, char **argv)
{
    if (argc != 2) {
        printf("Incorrect number of arguments.\n"
                "Usage: %s node\n"
                "where node is the IP address of a machine to be scanned.\n", argv[0]);
        return 1;
    }
    static struct addrinfo hints = { .ai_socktype = SOCK_STREAM };
    struct addrinfo *serviceinfo;
    if (getaddrinfo(argv[1], NULL, &hints, &serviceinfo) != 0) {
        puts("error in getaddrinfo() function call");
        return 1;
    }
    for (struct addrinfo* svc = serviceinfo; svc; svc = svc->ai_next) {
        for (unsigned port_no = 0; port_no < 1024; port_no++) {
            int sock_fd = socket(svc->ai_family, svc->ai_socktype, svc->ai_protocol);
            if (sock_fd == -1) {
                puts("error in socket() function call");
                freeaddrinfo(serviceinfo);
                return 1;
            }
            if (svc->ai_family == AF_INET) {
                ((struct sockaddr_in *)(svc->ai_addr))->sin_port = htons(port_no);
            } else if (svc->ai_family == AF_INET6) {
                ((struct sockaddr_in6 *)(svc->ai_addr))->sin6_port = htons(port_no);
            }
            if (connect(sock_fd, svc->ai_addr, svc->ai_addrlen) != -1) {
                printService(svc);
                printf("Port %u is open\n", port_no);
            }
            close(sock_fd);
        }
    }
    freeaddrinfo(serviceinfo);
}

Answer 2

1. Use getaddrinfo only once, to resolve the host. Do the port number iteration directly yourself. While it might be more complicated, it also doesn't call for repeated hostname resolutions. This also allows you to eliminate convert_int_to_string()
2. If you don't want IPv4 and IPv6 (which seems to be indicated by specifying AF_INET, you might be better off using the older functions. This eliminates the complexity noted before.
3. As the last step in the loop, you MUST close the socket. Otherwise, you will wind up with 1027 open descriptors. Since current linux systems have a limit of 1024 open descriptors, your program will terminate early without this. Other OSes will have similar limits. Since you only ever need 4 open descriptors (or maybe 5 depending on getaddrinfo()), you should do the cleanup.
4. I suggest localizing variables where possible.
5. In convert_int_to_string(), if you start at the end of the buffer and work backwards, you can return the point in the buffer that you have reached instead of reversing the text. Alternatively, snprintf(a, sizeof(a), "%d", num); works well.
6. Passing errno to error() is a mistake if errno isn't set. This includes the wrong number of arguments error (no number) and the call to getaddrinfo() (which returns an error number and has its own function to make it a string).