-1

I'm trying to use memcpy as a strncat since i need to preserve the null terminators in the strings I'm having a rough time trying to understand what i'm missing in this code, I don't know how to check it but i don't think the final string is correct, this is the code:

char *reass_tcp = malloc(sizeof(char) * 22 + 1);
char *a = "hel\0lo";
char *b = "W\0orld";
char *c = "wha\0ts\0up\r"; //last chunk of payload so it has the carriage 
                          //return which i included here

int len = 0;
memcpy(reass_tcp, a, actualsize(a));
len += actualsize(a);
memcpy(reass_tcp+len, b, actualsize(b));
len += actualsize(b);
memcpy(reass_tcp+len, c, actualsize(c));


return reass_tcp;

The original code is a bit more complex but it reduces to the problem above. I have access to a function actualsize(): this function returns the actual size of the string including the middle null terminators but not the one included in all C-strings (the one at the end of the string). It does however include the carriage return contained at the end of all tcp packets. e.g.

a = "hel\0lo";
actualsize(a) --> returns 6
c = "wha\0ts\0up\r"
actualsize(c) --> returns 10

UPDATE: this is what the scenario looks like:

  1. i have 3 tcp payload chunks where the last one only contains the carriage return
  2. i manually added a null terminator to all chunks
  3. my goal is to combine them, including the null terminators contained in-between the single strings, to form the original payload
ProdigySR
  • 11
  • 6
  • 2
    You need to (a) provide an [MCVE] (what you show clearly isn't, since there's a syntax error in the first line); (b) indicate what result you're getting, and what you expect/want. I'm also very dubious about you claims for `actualsize()`, so its code should be included. Even if it _does_ do what you claim it does, then at first glance it returns one less than it should... not counting the `NUL` at the end means it would get overwritten. Finally, the _source_ parameters in your `memcpy`s appear incorrect (3 x `tcp_payload` instead of, presumably, `a`, `b` and `c`). – TripeHound Aug 30 '20 at 21:48
  • Agreed, you can check quickly whether it can be reproduced here: https://www.onlinegdb.com/online_c_compiler – francis duvivier Aug 30 '20 at 21:53
  • yea sorry about 3). unfortunately this code is part of a larger codebase where the actualsize() function basically parses a tcp network packet retrieving the size of the payload, including the carriage return \r at the end of standard tcp packets. I've tested it myself so it basically returns strlen(payload) + 1 , where 1 is the carriage return – ProdigySR Aug 30 '20 at 21:59
  • `don't think the final string is correct` Define "*correct*" since you never say what you *expect* (or want) the end result to be. The way you have it now, `reass_tcp` will contain `hel\0loW\0orldwha\0ts\0up\r` without a nul-terminator at the end. – dxiv Aug 30 '20 at 22:18
  • hel\0loW\0orldwha\0ts\0up\r would be the correct output, however i don't receive the ack for the packet. If what i'm doing already results in the said correct string then concatenation is not the issue – ProdigySR Aug 30 '20 at 22:22
  • Yes, the code given here by itself seems to be fine, I tested it here: https://onlinegdb.com/H1DfOsYmw So your problem is probably gonna be somewhere else and you gonna have to find a minimal reproducible example in order to get help I'm afraid. – francis duvivier Aug 30 '20 at 22:29
  • thanks for double checking – ProdigySR Aug 30 '20 at 22:32
  • ProdigySR, How does the calling code know the length of data assigned in `reass_tcp`? Only pointer `reass_tcp` is returned. Code needs to return both the pointer and the length. – chux - Reinstate Monica Aug 31 '20 at 02:19

2 Answers2

0

Since you allocate space for a null terminator, you should set it explicitly afters the 3 memcpy calls by adding:

len += actualsize(c);
reass_tcp[len] = '\0';

If there are embedded null bytes in the payloads, reass_tcp would still be a C string, but if none of the 3 chunks have embedded null bytes, the terminator is required.

chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • Thanks for the quick reply. Each of the 3 chunks have the null byte at the end, that's why i didn't add it when combining them. If i add the null byte like you said i get memory corruption errors. I'll fix the question to make this more clear! – ProdigySR Aug 30 '20 at 21:46
  • You said `actualsize()` does not count the null terminator, hence you must add it manually as the last `memcpy` does not copy it. Chunks received via a tcp socket are usually not null terminated, so assuming there is a null terminator for the 3 chunks is not safe. The string literals do have a null terminator, but I assume they are provided for illustration only. – chqrlie Aug 30 '20 at 21:49
  • You are correct tcp chunks aren't usually null terminated that is why before sending the chunks i add them manually, that's why i know all the chunks are null terminated when i received them before concatenating them. – ProdigySR Aug 30 '20 at 21:53
  • Can you explain what magic trick `actualsize()` does to determine the number of bytes? – chqrlie Aug 30 '20 at 21:57
  • I updated the question hopefully it's explained a little better – ProdigySR Aug 30 '20 at 22:09
0

So turns out the code is correct the way it is, my error was allocating the wrong amount of space for the reassembled message before sending it as an actual network packet

ProdigySR
  • 11
  • 6