C Program infinite loop unexpectedly when used scanf

Question

I checked here and there and wasted around 3 hours checking for a solution. My program just infinite loop itself. Here is my C program:

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

int main (void)

{
    int attempts = 0;
    char password[10];

    do
    {
        printf("Enter your password:\n");
        scanf("%[^\n]s", password);
        printf("\n");
        attempts++;
    } while (strcmp(password, "awesome 123 ok"));

    printf("You entered a correct password in %d attempts!", attempts);

    return 0;
}

I tried scanf("%[A-Za-z0-9 ]s", password)"so it can take all characters and numbers including space as an input but it just loop. And I also tried using getchar() but it asks for password again and again even if I enter the correct one. Any help will be appreciated.

You don't need the `s` in the format `"%[^\n]s"`. Use just `"%[^\n]"`. — R Sahu, Aug 14 '14 at 06:15
Obligatory: Check return value of all `scanf` functions, always. — hyde, Aug 14 '14 at 06:17
Just check it out your scanf syntax. probably you can also use gets(password) or correct the scanf("%[^\n]", password); — Avinash, Aug 14 '14 at 06:17
@Avinash No no no, never advice anyone to use `gets`. It's prone to buffer overflows, have been deprecated in the C standard for long, and even removed in the latest C11 standard. — Some programmer dude, Aug 14 '14 at 06:18
@Avinash Never use `gets`! It's broken, deprecated, finally removed in C11. Use for example `fgets` or non-standard `getline` instead. — hyde, Aug 14 '14 at 06:20
@JoachimPileborg I just entered "C" tag and I'm not sure where other tags came from. — Amession, Aug 14 '14 at 06:28
@Amession Ah sorry, someone else edited the tags. I've removed it now. — Some programmer dude, Aug 14 '14 at 06:36

score 6 · Accepted Answer · answered Aug 14 '14 at 06:13

Size of awesome 123 ok is 15 including \0. But you are allocating memory for 10 bytes. It causes undefined behavior.

When you are using %[^\n] format specifier no need to use s with that, It will automatically scan the spaces also.

Try the following changes-

int main (void)

{
    int attempts = 0;
    char password[20]; // Fix 1

    do
    {
        printf("Enter your password:\n");
        scanf("%[^\n]", password); // Fix 2
        printf("\n");
        attempts++;
    } while (strcmp(password, "awesome 123 ok"));

    printf("You entered a correct password in %d attempts!", attempts);

    return 0;
}

2 problems: 1) If user enters `"\n"`, `password` will have random data in it. 2) On second loop, the first line's `"\n"` remains in `stdin`. blocking any new entry. Suggest `fgets()`. — chux - Reinstate Monica, Dec 29 '14 at 19:07

score 3 · Answer 2 · answered Aug 14 '14 at 06:13

3

You declare char password[10]; but you compare it with awesome 123 ok which has more characters.

answered Aug 14 '14 at 06:13

Alexandru Cimpanu

1,029
2
14
38

oh! this is embarrassing for me. Thanks for pointing that out :) – Amession Aug 14 '14 at 06:15

Some programmer dude · Answer 3 · 2014-08-14T06:24:40.750

From this scanf (and family) reference:

All conversion specifiers other than [, c, and n consume and discard all leading whitespace characters before attempting to parse the input.

That means that the trailing newline (a whitespace character) will be included in the next call to scanf in the loop.

The solution is simple: Tell scanf to read and discard leading whitespace:

scanf(" %[^\n]", password);
/*     ^              */
/*     |              */
/* Note leading space */

Also note that I removed the trailing s in the format, because that tells scanf to expect a literal s in the input. The "%[" format ends with the closing ']'.

You also might want to limit the number of characters read so you don't overflow the buffer you read into:

scanf(" %9[^\n]", password);

Note that the above format set the maximum field width as nine characters, because the buffer needs to include the terminating '\0' character as well. Modify this number if you increase the buffer size, but remember that it should be (at most) one less than the buffer size.

score 1 · Answer 4 · answered Aug 14 '14 at 06:17

1

Change the dec password[10] to password[20]

answered Aug 14 '14 at 06:17

Frankenstein

51
8

NPE · Answer 5 · 2014-08-14T06:28:26.027

The issue is that password is too narrow. Because of this, the program routinely writes past the end of the array, resulting in undefined behaviour.

You can fix this by making password wider. However, your program will still be open to stack smashing: an attacker can potentially execute arbitrary code by entering a long carefully crafted password string.

To fix that, you need to change the scanf() format specifier to limit how many characters it can store in password.

The following changes will fix both:

    char password[32]; /* more space: 31 characters + NUL */
    do {
        ...
        scanf("%31[^\n]%*c", password); /* format specifier */

The latter will make sure you're never reading more than 31 characters into password; it also consumes the newline without storing it.

What does the %*c do? is it same as %c? – Amession Aug 14 '14 at 06:25 — Amession, Aug 14 '14 at 06:25
@Amession: Yes, except it doesn't store it. – NPE Aug 14 '14 at 06:25 — NPE, Aug 14 '14 at 06:25

score 1 · Answer 6 · answered Aug 14 '14 at 06:27

I would use fgets because it is a safer way than gets or scanf:

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

int main (void) {
    int attempts = 0;
    char password[20];

    do {
        printf("Enter your password:\n");
        fgets(password, sizeof(password), stdin);
        password[strlen(password) - 1] = '\0';
        printf("\n");
        attempts++;
    } while (strcmp(password, "awesome 123 ok"));
    printf("You entered a correct password in %d attempts!", attempts);
    return 0;
}

Now, this will only work if you increase the size of password[] as I did in my example. When not using fgets it might still work in a bad way because you are comparing a buffer overflow.

Thanks, fgets is new for me, I learned something new today! :) — Amession, Aug 14 '14 at 06:31

score 0 · Answer 7 · answered Aug 14 '14 at 06:17

Add getchar() to consume the \n character. Find the modified code in below.

int main (void)

{
    int attempts = 0;
    char password[10];

    do
    {
        printf("Enter your password:\n");
        scanf("%[^\n]s", password);
        getchar (); // Fix1

        printf("\n");
        attempts++;
    } while (strcmp(password, "awesome 123 ok"));

    printf("You entered a correct password in %d attempts!", attempts);

    return 0;
}

The array password is not used properly in your code. So change the logic for it. User can enter N numbers of character. So restrict the user input to limited character or use dynamic memory allocation

C Program infinite loop unexpectedly when used scanf

7 Answers7