Segmentation Fault in TCP communication

Question

I'm getting a segmentation fault in the following C code:

#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <errno.h>
#include <string.h>
#include <netdb.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>

#define PORT  6667
#define MAXDATASIZE 1024

int bot_connect(char *hostname);

int bot_connect(char *hostname) {

  int sockfd, numbytes, s;
  char buf[MAXDATASIZE];
  struct addrinfo hints, *servinfo, *p;
  int rv;
  char m[1024];
  char *message;
  char *nick = "Goo";
  char *ident = "Goo";
  char *realname = "Goo";

  memset(&hints,0,sizeof hints);
  hints.ai_family = AF_UNSPEC;
  hints.ai_socktype = SOCK_STREAM;

  rv = getaddrinfo(hostname, PORT, &hints, &servinfo);

  if (rv != 0) {
    fprintf(stderr, "getaddrinfo: %s\n", gai_strerror(rv));
    return 1;
  }

  for (p = servinfo; p != NULL; p = p->ai_next) {
    sockfd = socket(p->ai_family, p->ai_socktype, p->ai_protocol);
    if (sockfd == -1) {
      perror("Client: socket");
      continue;
    }

    if (connect(sockfd, p->ai_addr, p->ai_addrlen) == -1) {
      close(sockfd);
      perror("Client: connect");
      continue;
    }

    break;
  }

  if (p == NULL) {
    fprintf(stderr, "Client: failed to connect \n");
    return 2;
  }

  freeaddrinfo(servinfo);

  strcat(m, "NICK ");
  strcat(m, nick);
  message = m;
  s = send(sockfd, message, strlen(message), 0);

  strcat(m, "USER ");
  strcat(m, ident);
  strcat(m, " * * :");
  strcat(m, realname);
  message = m;
  s = send(sockfd, message, strlen(message), 0);

  message = "JOIN #C&T";
  s = send(sockfd, message, strlen(message), 0);

  close(sockfd);
}

I know that you get segmentation faults from trying to do something with memory that you are not allowed to do, like alter read only memory, but to my knowledge, this program doesn't do that. Does anyone have any clue where the segmentation fault is coming from?

Answer 1

You are calling strcat( m, "NICK" ); before you have initialized m. before the strcat, try m[0] = '\0'; or memset( m, 0, sizeof( m ) );, or change the first strcat to strcpy

Also, after you send the NICK line out on the socket, you call strcat again, which will append the USER line to the NICK line. Again, you should probably change the first strcat to strcpy

Answer 2

Graeme is correct with the "what", so here's the "why" in case you're unfamiliar. In C, a string is defined as a series of characters terminated by a null character '\0'. The reason that a string has a terminating character is so that code can figure out where the string logically ends. Even if you declare a string like this:

 char m[1024];

There is no way in C that code given m can figure out that it has 1024 bytes allocated to it, let alone figure out how many of those bytes are meaningful, without having a null character that demarcates the end of the meaningful bytes.

strcat is a function that works on two C strings, therefore it expects both of its arguments to conform to the specification that there is a null byte marking the end of meaningful input. If m is uninitialized, its contents are random, so the null byte could be anywhere, or there could be no null byte at all. In order to concatenate, strcat will look through the characters of the first argument trying to find the null byte that marks the end of the string. If, by chance, no null byte exists in the uninitialized array, it will happily continue looking past the end of the string, into arbitrary memory locations that may correspond to other variables, or internal stack information, etc.

When strcat finally finds a null byte, it will happily write the contents of the second argument starting at that location, and if that location is past the end of m, it will overwrite some other variable or other piece of information. If its a memory address that is assigned to some other variable, the problem can be silent and very tough to find. Luckily, in your case it was a location that shouldn't be written to, so your program crashed in an obvious error.

This also brings up the note that you really should be using strncat instead of strcat. strncat lets you specify the maximum length of the string to create. I noted before that strcat cannot determine that m has 1024 bytes allocated to it, so if you try to concatenate m with something that would make m longer than 1023 characters, you end up with the same memory-overwriting problem. strncat takes a third argument that specifies the maximum length of the resultant string and has a return code that will indicate if the concatenation was truncated due to reaching this length.

Answer 3

Here are already the right answers to the why and what. To answer your "where the segmentation fault..." I recommend a debugger. Compile your programm with debug info, and you will see where it happens (just let it run and look at the stack trace when it crashes). The answer will most likely also answer the why and what (which is in this case already answered)

Answer 4

Turn up the warning level on your compiler:

rv = getaddrinfo(hostname, PORT, &hints, &servinfo);

The second parameter to getaddrinfo is a const char *, not an int. Change PORT to NULL and modify the port number in the addrinfo structure before the connect.

Answer 5

Use valgrind. Valgrind is extremely good in finding resource leaks and other common human error-type of misbehaviour in your code.

Answer 6

The m array contains garbage before use. The following would work:

strcpy(m, "NICK ");
strcat(m, nick);
message = m;
s = send(sockfd, message, strlen(message), 0);

strcpy(m, "USER ");
strcat(m, ident);
strcat(m, " * * :");
strcat(m, realname);
message = m;
s = send(sockfd, message, strlen(message), 0);

Also the following would work:

sprintf(m, "NICK %s", nick);
message = m;
s = send(sockfd, message, strlen(message), 0);

sprintf(m, "USER %s * * :%s", ident, realname);
s = send(sockfd, message, strlen(message), 0);