Segmentation Fault while using wrefresh function of ncurses in C

Question

First of all, I create two windows with ncurses : one for transmission and one for reception. Basically, one to write the command and the other to print the command. To write the input in the reception window i did a little function to concatenate the string already in the reception window and the one written by the user (i think the problem is in there).

So, when I run the code, the program fails and displays Segmentation Fault code dumped at the line wrefresh(winReception);

But the weird thing is, if the input is 7 characters or less it works, if it's 8 or more it breaks.

I'm using Code::Blocks

Screenshots here : https://i.stack.imgur.com/JTS12.jpg

Here is some code :

//Global variables
WINDOW * winReception;
WINDOW * winTransmission;
char * command;
char mesg[] = "Enter a command";
//variable to stock the input in reception window
char *textinwindow = "";

//main
int main(int argc, char* argv[])
{
    initscr();

    /* WINDOW RECEPTION */
    winReception = newwin(15, 0, 0, 0);
    wrefresh(winReception);


    /* WINDOW TRANSMISSION*/
    winTransmission= newwin(8, 0, 15, 0);
    wrefresh(winTransmission);
    mvwprintw(winTransmission, 1, 2, mesg);

    wgetstr(winTransmission, &command);
    verifInput(&command);

    free(textinwindow);

    exit(0);
}

//concat function (where I think the bug is)
char* concat(char *s1, char *s2)
{
    char *result = (char *) malloc(strlen(s1) + strlen(s2) + 1);
    strcpy(result, s1);
    strcat(result, "\n ");
    strcat(result, s2);

    return result;
}

//verifInput (where the program fails)
void verifInput (char* cmd)
{
    /* WINDOW RECEPTION */
    textinwindow = concat(textinwindow, cmd);
    mvwprintw(winReception, 1, 2, textinwindow);
    wrefresh(winReception);

    /* WINDOW TRANSMISSION*/
    touchwin(winTransmission);
    wclear(winTransmission);
    wrefresh(winTransmission); //Program fails here
    mvwprintw(winTransmission, 1, 2, mesg);

    wgetstr(winTransmission, &command);
    verifInput(&command);
}

Answer 1

There were a number of problems here as noted in the comments, but the main one was the completely incorrect use of wgetstr() because it's accepting input from the user and stuffing it into a buffer, but you have not allocated a buffer. We don't know what the value of command is, so it's storing data into random memory.

The wrong way to fix this is:

char command[256];
wgetstr(winTransmission, command); // NO

because though you'd be providing a place to store the user's input, wgetstr() doesn't know how big the buffer is, and if the user types too much, it's going to overwrite the memory there too. Not good.

Instead, we'll use a bounded version wgetnstr() that takes a buffer and a count

char command[256];
wgetnstr(winTransmission, command, sizeof command); // YES

Now it will never overwrite the buffer!

Other issues: though you're freeing the textinwindow memory at the end of the code, every time you call concat() the old value of textinwindow - which is also allocated memory - is thrown away. That's a for-sure memory leak.

Finally, a subtle issue. Since mvwprintw() takes a printf-style format string, the value you pass it is coming from the user and could contain a %s token. No good comes from this. Instead:

    mvwprintw(winReception, 1, 2, "%s", textinwindow);

which means any funky % in the user input string won't cause havoc.

This is what I came up with:

#include <stdio.h>
#include <ncurses.h>
#include <string.h>
#include <stdlib.h>

//Global variables
WINDOW * winReception;
WINDOW * winTransmission;

char command[256];

const char mesg[] = "Enter a command";
//variable to stock the input in reception window
char *textinwindow = 0;

void verifInput (const char* cmd);

//main
int main(int argc, char* argv[])
{
    initscr();

    /* WINDOW RECEPTION */
    winReception = newwin(15, 0, 0, 0);
    wrefresh(winReception);


    /* WINDOW TRANSMISSION*/
    winTransmission= newwin(8, 0, 15, 0);
    wrefresh(winTransmission);
    mvwprintw(winTransmission, 1, 2, mesg);

    wgetnstr(winTransmission, command, sizeof command);
    verifInput(command);

    free(textinwindow);

    exit(0);
}

//concat function (where I think the bug is)
char* concat(const char *s1, const char *s2)
{
    // 2 = newline + space
    // 1 = final NUL byte
    char *result = (char *) malloc(strlen(s1) + 2 + strlen(s2) + 1);
    strcpy(result, s1);
    strcat(result, "\n ");
    strcat(result, s2);

    return result;
}

//verifInput (where the program fails)
void verifInput (const char* cmd)
{

    // free up old memory except for the first time
    if (textinwindow == 0)
        textinwindow = concat("", cmd);
    else
    {
        char *save = textinwindow;
        textinwindow = concat(textinwindow, cmd);
        free(save);
    }

    /* WINDOW RECEPTION */
    mvwprintw(winReception, 1, 2, "%s", textinwindow);
    wrefresh(winReception);

    /* WINDOW TRANSMISSION*/
    touchwin(winTransmission);
    wclear(winTransmission);
    wrefresh(winTransmission); //Program fails here
    mvwprintw(winTransmission, 1, 2, mesg);

    wgetnstr(winTransmission, command,  sizeof command);
    verifInput(command);
}