recv overwrite a char[]

Question

Hi all, I'm trying to make a little client-server script like many others that I've done in the past.
But in this one I have a problem. It is better if I post the code and the output it give me.
Code:

#include <mysql.h> //not important now
#include <stdlib.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <netdb.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <signal.h>
#include <string.h>

//constant definition
#define SERVER_PORT 2121
#define LINESIZE 21

//global var definition
char victim_ip[LINESIZE], file_write[LINESIZE], hacker_ip[LINESIZE];

//function
void leggi (int); //not use now for debugging purpose
//void scriviDB (); //not important now

main () {

int sock, client_len, fd;

struct sockaddr_in server, client;

// transport end point
if((sock = socket(AF_INET, SOCK_STREAM, 0)) == -1) {
  perror("system call socket fail");
  exit(1);
}

server.sin_family = AF_INET;
server.sin_addr.s_addr = inet_addr("10.10.10.1");
server.sin_port = htons(SERVER_PORT);

// binding address at transport end point
if (bind(sock, (struct sockaddr *)&server, sizeof server) == -1) {
  perror("system call bind fail");
  exit(1);
}

//fprintf(stderr, "Server open: listening.\n");
listen(sock, 5);

/* managae client connection */
while (1) {
  client_len = sizeof(client);
  if ((fd = accept(sock, (struct sockaddr *)&client, &client_len)) < 0) 
     { perror("accepting connection"); exit(1);  }

  strcpy(hacker_ip, inet_ntoa(client.sin_addr));
  printf("1 %s\n", hacker_ip); //debugging purpose
  //leggi(fd);

//////////////////////////
//receive client 
  recv(fd, victim_ip, LINESIZE, 0);
  victim_ip[sizeof(victim_ip)] = '\0';
  printf("2 %s\n", hacker_ip); //debugging purpose
  recv(fd, file_write, LINESIZE, 0);
  file_write[sizeof(file_write)] = '\0';
  printf("3 %s\n", hacker_ip); //debugging purpose
  printf("%s@%s for %s\n", file_write, victim_ip, hacker_ip);

  //send to client
  send(fd, hacker_ip, 40, 0); //now is hacker_ip for debug

/////////////////////////

  close(fd);

}//end while

exit(0);
} //end main

Client send string: ./send -i 10.10.10.4 -f filename.ext
so the script send -i (IP) and -f (FILE) at the server.
Here's my output server side:

1 10.10.10.6
2 10.10.10.6
3
[email protected] for

As you can see the printf(3) and the printf(ip,file,ip) fail.
I don't know how and where but someone overwrite my hacker_ip string.
Thanks for your help! :)

Answer 1

TCP provides a stream, not packets. Thus, you are not guaranteed that the data you send with 1 send() call takes 1 recv() call to receive. It could take several recv() calls to receive what one send() call sent, or it could take 1 recv() call to receive what several send() calls sent - somehow you have to handle that.

In particular, you should check the return value of recv() to know how many bytes you received, maybe this can be a start so you are atleast not printing garbage in your strings.

ssize_t bytes = recv(fd, victim_ip, LINESIZE, 0);
 if(bytes == 0) {
   //remote closed the connection, handle it
 } else if (bytes < 0) {
    //handle error
 } else {
   victim_ip[bytes] = '\0';
   printf("%s\n", victim_ip); 
}

Answer 2

You should correct lines like:

victim_ip[sizeof(victim_ip)] = '\0';

and

file_write[sizeof(file_write)] = '\0';

This is this one that overwrite the hacker_ip string.

It write a zero after the end of the array (sizeof(file_write) == LINE_SIZE). If you want to write a watchdog zero you should set dimension of array to one more char like file_write[LINE_SIZE+1].

This excepted, it should work. For very small data chunks like here (21 bytes), it is unlikely that the packet will be splitted (standard ethernet frames are about 1400 bytes). But if you did several send they will certainly be merged in the same packet.

It would be interresting to see the sender code. Did you send a full buffer every time ? (should be looking at your recv()).