Search code examples
cforkpipe

Pipe overwrites buffer, don't know how to overcome


I use a simple pipe. I read with a while, 1 char at a time, I think every time I read a char I overwrite something

#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/wait.h>
#include <string.h>

int main () {
    int pipefd[2];
    int cpid;
    char buf[31];
    if (pipe(pipefd) == -1) {
        perror("pipe");
        exit(EXIT_FAILURE);
    }
    cpid = fork();
    if (cpid == -1) {
        perror("cpid");
        exit(EXIT_FAILURE);
    }
    if (cpid == 0) {      // child reads from pipe
        close (pipefd[1]); // close unused write end
        int i = 0;
        while (read (pipefd[0], &(buf[i++]), 1)>0);
        printf ("Server receives: %s", buf);
        close (pipefd[0]);
        exit (EXIT_SUCCESS);
    }
    else {               // parent writes to pipe
        close (pipefd[0]); // closing unused read end;
        char buf2[30];
        printf("Server transmits: ");
        scanf ("%s", buf2);
        write (pipefd[1], buf2, strlen(buf2)+1);
        close(pipefd[1]);
        wait(NULL);
        exit(EXIT_SUCCESS);
    }
  return 0;
}

Code is now fixed, this is obsolete: For example, if I input: "Flowers" it prints F and then ~6 unprintable characters

BUT: A small odd thing that's happening, I just used a string way longer than 30, and it produced no error at all, and it did manage to write the entire string. Though both my buffers are considerably smaller than that.


Solution

  • (Following Anders' suggestion.)

    Using GCC 4.6.3, my code:

    #include <unistd.h>
    #include <stdio.h>
    #include <stdlib.h>
    #include <sys/wait.h>
    #include <string.h>
    
    int main () {
        int pipefd[2];
        int cpid;
        char buf[31];
        if (pipe(pipefd) == -1) {
            perror("pipe");
            exit(EXIT_FAILURE);
        }
        cpid = fork();
        if (cpid == -1)
        {
            perror("cpid");
            exit(EXIT_FAILURE);
        }
        if (cpid == 0) {      // child reads from pipe
            close (pipefd[1]); // close unused write end
            int i=0;
            while (read(pipefd[0], &(buf[i++]), 1) != 0);
            printf ("Server receives: %s\n", buf);
            close (pipefd[0]);
            exit (EXIT_SUCCESS);
        }
        else {               // parent writes to pipe
            close (pipefd[0]); // closing unused read end;
            char buf2[30];
            printf("Server transmits: ");
            scanf ("%s", buf2);
            write (pipefd[1], buf2, strlen(buf2)+1);
            close(pipefd[1]);
            wait(NULL);
            exit(EXIT_SUCCESS);
        }
      return 0;
    }
    

    Produces:

    [user@host tmp]$ gcc pipes.c -o pipes && ./pipes 
    Server transmits: Flowers
    Server receives: Flowers
    

    (I also agree with his sentiment about bounds checking.)


    EDIT: Per your comment, if you change the following line (35 for me)

    scanf("%s", buf2);
    

    to

    fgets(buf2, 30, stdin);
    

    You gain two benefits. (a) You eliminate a buffer overflow vulnerability by limiting the number of bytes that will be copied into buf2. (b) You are able to "accept" non-newline whitespace (spaces and tabs) whereas with scanf, you were not:

    [user@host tmp]$ gcc pipes.c -o pipes && ./pipes 
    Server transmits: Flowers smell nice
    Server receives: Flowers smell nice