1

I have defined a char array with a predefined HTTP post string like this:

char header[] = "POST /api/add HTTP/1.1\r\nHost: xxxxxxx:3000\r\nContent-Type: application/octet-stream; charset=utf-8\r\nContent-Length: 500\r\nName: ";

strcat(header, strDevicename); \\
strcat(header, "\r\n\r\n");

where the strDevicename is a char variable name changed every request. The problem is when I run it for the first time, it is working but after that getting overwritten Name with a96ed5ÿÿa96ed58e8355.

What is the best way to add two string with one real-time change variable using C languages in HTTP post header?

2
  • You need to create a buffer big enough to hold the two strings plus the NULL-terminator Commented Jul 26, 2018 at 15:16
  • ...and you should read the chapter dealing with strings in your C text book. Commented Jul 26, 2018 at 15:19

5 Answers 5

4

In your code, the size of the array header is decided by the size of the supplied initializer string, and it does not have any additional space to store (or append) any further characters.

Quoting C11, chapter §6.7.9

If an array of unknown size is initialized, its size is determined by the largest indexed element with an explicit initializer. The array type is completed at the end of its initializer list.

Next, for strcat(), from chapter §7.24.3.1

The strcat function appends a copy of the string pointed to by s2 (including the terminating null character) to the end of the string pointed to by s1. [...]

which indicates, the target s1 (here, header) should have sufficient storage to hold the concatenated string.

Thus, the attempt to strcat() with header as source, invokes undefined behaviour here, as you run past allocated memory.

You need to make header have enough space left after you fill it with the initializer string. Use a fixed size for the array, which has much excess after filling it with the initialize string, something like

#define STRSIZ 512

char header[STRSIZ] = "POST /api/add HTT.........
Sign up to request clarification or add additional context in comments.

Comments

2

From man 3 strcat:

The strings may not overlap, and the dest string must have enough space for the result. If dest is not large enough, program behavior is unpredictable; buffer overruns are a favorite avenue for attacking secure programs.

You need to ensure your header array is allocated large enough to ensure that you can write strlen(strDeviceName) + 5 bytes into it in addition to the initial size; otherwise you have a (probably remotely exploitable) buffer overrun vulnerability.

Presumably header is allocated locally to the function? In that case you should consider using alloca or malloc to get a properly-sized block of memory rather than relying on a static size. You'll also need to handle errors from those functions.

Additionally, you should always prefer the safe alternative strncat over plain strcat, as strncat takes an additional argument for the number of bytes to append, and ensures that the buffer is always null-terminated even if an overflow would otherwise have happened.

Comments

0

The problem is that you are [str]concatenating to an array (header) which doesn't extra space (C arrays can't be changed in size).

snprintf could be a better fit here.

If you know the maximum possible length of strDevicename, you could use a fixed size buffer:

const char header[] = "POST /api/add HTTP/1.1\r\nHost: xxxxxxx:3000\r\nContent-Type: application/octet-stream; charset=utf-8\r\nContent-Length: 500\r\nName: ";
const char tail[] = "\r\n\r\n";
char buf[(sizeof header - 1) + MAX_DEV_LENGTH + (sizeof tail - 1) + 1];
/* sizeof would count the null byte in header & tail arrays. */
snprintf(buf, sizeof buf, "%s%s%s", header, strDevicename, tail);

If strDevicename is of unknown length, you could use asprintf(GNU function):

char *buf = NULL;
if (asprintf(&buf, "%s%s%s", header, strDevicename, tail) == -1) {
   /* handle memory allocation failure */
}

...

free(buf);

If you asprintf isn't available, then you could allocate sufficient memory (just as above) yourself using malloc and then use snprintf.

7 Comments

Don't you think that sprintf function is an overkill for the sting concatenation ?
Possibly. Arguable if it's worse than N(=2 here) strcat calls (strcat is also vulnerable to buffer overflows. So in the "fixed size buffer" approach, a call to strlen becomes necessary to ensure the length of strDevicename). But if I am worried about performance, I'd measure first.
printf is a very heavy function. Needs to parse the format string first. And not everyone writes programs for the computers with (almost) infinitive resources. Imagine small uC with 4k Flash and printf will take 3.5k..
And how much memory you'd need for a HTTP request & response on such systems? ( that's likely what OP does).
Overkill or not -- I don't think the OP is at the performance optimization step yet.
|
0

You need to allocate sufficient space for the entire string. Man page:

char * strcat(char *restrict s1, const char *restrict s2);
The string s1 must have sufficient space to hold the result.

You could do it the "right" way and allocate exactly what you need:

char *buf;
char header[] = "POST /api/add HTTP/1.1\r\nHost: xxxxxxx:3000\r\nContent-Type: application/octet-stream; charset=utf-8\r\nContent-Length: 500\r\nName: "

buf = malloc(strlen(header)+strlen(strDeviceName)+strlen("\r\n\r\n")+1);
if(buf==NULL) abort();
strcpy(buf, header);
strcat(buf, strDevicename); \\
strcat(buf, "\r\n\r\n");

Or do it the lazy way and overallocate:

char header[1024] = "POST /api/add HTTP/1.1\r\nHost: xxxxxxx:3000\r\nContent-Type: application/octet-stream; charset=utf-8\r\nContent-Length: 500\r\nName: "

strcat(header, strDevicename); \\
strcat(header, "\r\n\r\n");

1 Comment

Why the downvote? Any advice on correcting this answer?
0

As you need to have enough space you can duplicate both in the new one:

char *mystrcat(const char *src1, const *char src2)
{
    char *dest
    size_t src1Len = strlen(src1);
    dest = malloc(src1Len + strlen(src2) + 1);
    if(dest)
    {
        memcpy(dest, src1, src1Len);
        strcpy(dest + src1Len, src2);
    }
    return dest;
}

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.