C check user input error

Question

All I am trying to do is ask for a password and print out an error message if the input is longer than 10 characters. It works if the first input is shorter than 10 characters. It just prints out the input and exits the program. If the input is longer than 10 characters the error message prints and it asks for a new password, but if the input is less than 10 characters on the second try it prints out the input and then the program breaks with a "Thread:1 signal SIGABRT" error. I know that I shouldn't be using gets, but I am trying to find a way to make my code work using it.

#include <stdio.h>

#define BUFFER_LENGTH   11

int main() {
    int cont;
    while (1) {
        char line[BUFFER_LENGTH];
        char *p;
        printf("Enter Password: ");
        p = gets (line);
        if (strlen(p)>10) {
            printf("Error! Password must be shorter than 10 characters! \n");
        }else{
            printf(p);
            printf("\n");
            break;
        }
    }
}

Answer 1

If user input is longer than 10 characters, you end up using memory beyond the valid limits. That's exactly the reason you MUST avoid using gets. See Why is the gets function so dangerous that it should not be used? for more info on the subject.

Change the gets line to:

fgets(line, sizeof(line), stdin);

Then, you don't have to worry about the user entering more than 10 characters. They will be simply ignored.

If you want to deal with that use case as a user error, change the size of line but still use fgets.

Update, thanks to @chux

If the user enters less than the 11 characters in your case, the line

 fgets(line, sizeof(line), stdin);

will not only read the characters, it will also include the ending newline character in it. You'll have to add a bit of code to trim the newline character from line.

 // Trim the newline character from user input.
 size_t len = strlen(line);
 if ( len > 0 && line[len-1] == '\n' )
 {
    line[len-1] = '\0';
 }

Answer 2

To detect if more than n characters are entered, code must

1. Read at least n+1 non-control characters.

2. Handle excessive characters.

OP's code should not use gets(). It is an obsolete function and does not prevent buffer overrun.

void GetPrintPW(void) {
  // +1 for the null character.
  char pw[PASSWORD_MAX_LENGTH + 1];

  size_t i = 0;
  bool too_long = false;
  int bad_char = EOF;
  int ch;

  // Note: All characters in the line are consumed. Only the first `n` are saved.
  while ((ch = fgetc(stdin)) != '\n' && ch != EOF) {

    // This would be a good place to add code to check if the character is "good".      
    if (!isprint(ch)) bad_char = ch;

    if (i < PASSWORD_MAX_LENGTH) pw[i++] = ch;
    else too_long = true;
  }
  pw[i] = '\0';

  if (bad_char != EOF) {
     printf("Error! Bad character, code %d\n", bad_char);
  }  else if (too_long) {
     // Avoid `printf(only_some_string)`
     puts("%Error! Password must be shorter than 10 characters!");
  } else {
    // this is BAD! as a % in pw will cause UB with `printf()` 
    // printf(pw);
    printf("'%s'\n", pw);
  }

  // Always a good idea to scrub data after using a password to prevent memory snooping.
  memset(pw, 0, sizeof pw);
}

---

PW notes: Do not use getline() for reading passwords as code loses control over buffers where the password was stored. Use a plain char array and scrub afterwards. Using malloc(), realloc() etc. can also have similar problems.

Good OS's will have a special function to read passwords as at each function level, any buffer data needs to be scrubbed.

Answer 3

If you use gets, line needs to be big enough to hold all the possible characters a user could enter before hitting return. How many is that? The limit approaches infinity. Use fgets instead.