C strncat not working and raises Access Violation

Question

I am writing a web server, in which I recieve the data from a client through a WinSock socket, and parse the data into parts. Then, according to the method found and the resource requested, I want to build a new packet which I will send back to the client.

I have this function, used to build the packet to send nack to client: (BUFFER_SIZE=2048)

char* build_packet(char *ver, char *code, char *content, long length, long *p_length)
{

    char *packet = (char *)malloc(BUFFER_SIZE);
    char *len_str = malloc(BUFF_64);
    _itoa(length, len_str, 10);
    char *content_len = "Content-length: ";
    char *content_type = "Content-Type: ";
    long len = strlen(ver) + 1;
    len += strlen(code) + strlen(HTTP_DELIM);
    len += strlen(content_len) + strlen(len_str) + strlen(HTTP_DELIM);
    len += strlen(content_type) + strlen(HTTP_HTML_TYPE_TEXT) +strlen(HTTP_DELIM);
    len += strlen(content);
    *p_length = len;


// This is where problems start...

// ##############################################################

    strncat(ver, packet, BUFFER_SIZE - 1); // Trouble starts here
    strncat(" ", packet, BUFFER_SIZE - 1);
    strncat(code, packet, BUFFER_SIZE - 1);
    strncat(HTTP_DELIM, packet, BUFFER_SIZE - 1);
    strncat(content_len, packet, BUFFER_SIZE - 1);
    strncat(len_str, packet, BUFFER_SIZE - 1);
    strncat(HTTP_DELIM, packet, BUFFER_SIZE - 1);
    strncat(content_type, packet, BUFFER_SIZE - 1);
    strncat(HTTP_HTML_TYPE_TEXT, packet, BUFFER_SIZE - 1);
    strncat(HTTP_DELIM, packet, BUFFER_SIZE - 1);
    strncat(content, packet, BUFFER_SIZE - 1);

    return packet;

}

(Of course I should (and I will) add validation to the strncat so that it will eventually send all data)

My question is: why is the first strncat call not working and raises a Access Violation error? What am I doing wrong?

Assuming that you are attempting to append to `packet` the order of your arguments are wrong. Please see e.g. [this `strncat` reference](http://en.cppreference.com/w/c/string/byte/strncat). Also remember that `strncat` relies on the destination buffer to already be terminated, and that `malloc` leaves the memory it allocates uninitialized. — Some programmer dude, Jan 16 '17 at 12:28
This `strncat(" ", packet, BUFFER_SIZE - 1);` is wrong - you are attempting to modify a string literal (*undefined behaviour*). You probably do the same with others (such a `ver` etc). — P.P, Jan 16 '17 at 12:29
@Someprogrammerdude so should I use `memset` to set the memory to 0's and then `strncat`? — Ofer Arial, Jan 16 '17 at 12:32
See: [How to concatenate const/literal strings in C?](http://stackoverflow.com/q/308695/4389800). — P.P, Jan 16 '17 at 12:32
@usr I'm trying to append a space character(as a string here) and other strings such as ver (char*) to my packet. Is it not the write way to do so? — Ofer Arial, Jan 16 '17 at 12:33
You have four alternatives: One is to use `memset` as you suggest. Another is to use `calloc` to allocate the memory instead, it combines `malloc` and `memset` into a single call. It simpler and more effective to start with a single `strcpy` call instead of `strcat`. Or why not use a single call to `snprintf` to format the whole string in one go? — Some programmer dude, Jan 16 '17 at 12:34
@OferArial `" "` is a string literal. So, you can't append to it. Use an array or malloc'ed buffer and then use `snprintf()` - which is probably easier and safer compared to strcpy() and strncat() calls. — P.P, Jan 16 '17 at 12:36

Gerhardh · Accepted Answer · 2017-01-16T12:43:15.457

2

You mix up your parameters for strncat. First paramter should be the destination buffer.

strncat(ver, packet, BUFFER_SIZE - 1); // Trouble starts here
strncat(" ", packet, BUFFER_SIZE - 1);
...

You add the same random (and not nul terminated) data from packet to various other buffers that are of different size.

This should be

strncat(packet, ver, BUFFER_SIZE - 1); // Trouble starts here
strncat(packet, " ", BUFFER_SIZE - 1);
...

You should also initialize packet first:

packet[0] = 0;
strncat(packet, ver, BUFFER_SIZE - 1);
strncat(packet, " ", BUFFER_SIZE - 1);
...

edited Jan 16 '17 at 12:43

answered Jan 16 '17 at 12:31

Gerhardh

11,688
4
17
39

3

`strncpy` is dangerous, it will not 0-terminate `packet` if `strlen(ver) > BUFFER_SIZE -1`. – mch Jan 16 '17 at 12:33
@mch: agreed. I will remove this version. – Gerhardh Jan 16 '17 at 12:42
1

Also note that the third argument is the maximum size of the string to be appended, not the maximum size of the buffer being appended to. So this usage will not protect you from a buffer overrun. – rici Jan 16 '17 at 14:13
Thank you all very much, now I found a way to use it safely and successfully :) – Ofer Arial Jan 17 '17 at 19:04

score 2 · Answer 2 · answered Jan 17 '17 at 14:01

2

It seems you are misunderstanding the semantics of strncat():

The first argument is a pointer to the destination array.
The second argument is a pointer to the source string or array,
The third argument is the maximum number of characters to copy from the source, not the maximum length of the destination string.

answered Jan 17 '17 at 14:01

chqrlie

131,814
10
121
189

Thank you very much, you helped :) – Ofer Arial Jan 17 '17 at 19:04

C strncat not working and raises Access Violation

2 Answers2