How do I properly utilize fgets() and strncmp() functions?

Question

I am trying to secure some C code by utilizing the fgets() and strncmp() functions. The program runs fine, however, if I enter the correct password ("password") more than once it still indicates a correct password. Also, even when using fgets() the results (if longer than the 9 indicated in the buffer) still appear in the output. Any help would be greatly appreciated.

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

int main(void)
{
    char buffer[9];
    int pass = 0;
    char password[] = "password";

    printf("\n Enter your password : \n");
    fgets(buffer, 9, stdin);

    if(strcmp(buffer, password))
    {
        printf ("\n Incorrect Password \n");
    }
    else
    {
        printf ("\n Correct Password \n");
        pass = 1;
    }

    if(pass)
    {
        printf ("\n Root privileges authorized \n");
    }

    return 0;
}

Answer 1

The problem with your code is that fgets is taking the first 8 characters off the input and ignoring the rest. Obviously, if you are inviting a password you don't want to ignore any input! You might want to do something a little more fancy to ensure that you capture the full input.

My first two tries at answering this were wrong. Thanks to wildplasser for holding my feet to the fire.

So, the hack answer is: use a really big buffer. fgets is probably your easier solution there.

Alternatively, you could allocate memory dynamically as your input string exceeds your buffer.

But, just for fun, here is an implementation that breaks us out of the "line buffer" trap that I wasn't aware getchar was in.

For this, I leveraged a very beautiful comment here: getchar() and stdin

PS: Ok, Ok, I tested it this time. It works. On my Mac.

#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <termios.h>

int main(void)
{
    int c, i;
    char buffer[9];
    struct termios tty_opts_default, tty_opts_raw;

    if (!isatty(STDIN_FILENO)) {
      printf("Error: stdin is not a TTY\n");
      return 1;
    }

    /* save tty settings for later. */
    tcgetattr(STDIN_FILENO, &tty_opts_default);

    /* put tty settings into raw mode. */
    cfmakeraw(&tty_opts_raw);
    tcsetattr(STDIN_FILENO, TCSANOW, &tty_opts_raw);

    /* NOW we can grab the input one character at a time. */
    c = getchar();
    while (i < 8 && c != EOF && c != '\n' && c != '\r') {
      /* Since we are collecting a pwd, we might want to enforce other
         password logic here, such as no control characters! */
      putchar('*');
      buffer[i++] = c;
      c = getchar();
    }
    buffer[i] = '\0';

    /* Restore default TTY settings */
    tcsetattr(STDIN_FILENO, TCSANOW, &tty_opts_default);

    /* Report results to user. */
    printf("\nPassword received.\n");
    printf("(It was '%s' -- don't tell anyone! Quick! Hide the terminal!)\n", buffer);

    return 0;
}

Answer 2

• fgets()reads the (CR+)LF too, and stores it into the buffer.

• But only if there is sufficient place!

• Otherwise, your buffer will contain the first n-1 characters, plus a NUL character.

  So: allocate a large-enough buffer, and strip the CR/LF:

---

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

int main(void)
{
    int pass = 0;
    char password[] = "password";
    char buffer[3+ sizeof password];

    printf("\n Enter your password : \n");
    fgets(buffer, sizeof buffer, stdin);
    buffer[strcspn(buffer,"\r\n")]=0;

    if(strcmp(buffer, password))
    {
        printf ("\n Incorrect Password \n");
    }
   
    else
    {
        printf ("\n Correct Password \n");
        pass = 1;
    }

    if(pass)
    {
        printf ("\n Root privileges authorized \n");
    }

    return 0;
}