1

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;
        }
    }
}
Improve this question
4
  • 4
    Don't use gets() which has been removed from C standard. use fgets() instead. Commented Jan 29, 2016 at 22:31
  • 1
    You've allocated a bare minimum buffer for gets. If the user enters more than 10 characters, then gets will write past the end of line. If you use fgets, then you can tell it how big your buffer is, which is the only way to protect against this problem (using a larger buffer merely makes the error less likely). Commented Jan 29, 2016 at 22:36
  • When I use fgets() and the input is longer than 10 char it prints the error message and then takes the characters after the first 10 and prints those out. That isn't what I want. Is there a way to get fgets() to work for my program? Commented Jan 29, 2016 at 22:38
  • 1
    printf(p); never ever ever do this! printf("%s", p); instead. Commented Jan 29, 2016 at 22:42

3 Answers 3

Reset to default
3

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';
 }
Improve this answer
Sign up to request clarification or add additional context in comments.

7 Comments

If you use this and the input is longer than 10 characters (ex: abcdefghijkl) then it prints out the error message and then takes the remaining characters after the first 10 (ex: kl) and prints those out as valid input. Is there a way to erase the data in "line" so that it doesn't look for the remaining characters?
Sure, you can write int c; while ( (c = fgetc(stdin)) != EOF && c != '\n'); to ignore the rest of the line.
No, extra characters won't be ignored, they will stay around to get picked up by the next read.
fgets(line, sizeof(line), stdin); needs additional work to insure the password is not too long. It does prevent user input overflow, but line may contain a final '\n' or not.
Note: line[len-1] is a hacker exploit as the first char read by fgets() may be a null character which leads to line[0-1] and UB. Best to check len > 0 first or stackoverflow.com/a/2693827/2410359
|
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.

Improve this answer

Comments

0

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.

Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.