Amession
Amession

Reputation: 249

C Program infinite loop unexpectedly when used scanf

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.

Upvotes: 1

Views: 576

Answers (7)

NPE
NPE

Reputation: 500883

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.

Upvotes: 1

Jens Wirth
Jens Wirth

Reputation: 17460

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.

Upvotes: 1

Some programmer dude
Some programmer dude

Reputation: 409442

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.

Upvotes: 3

mahendiran.b
mahendiran.b

Reputation: 1323

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

Upvotes: 0

Frankenstein
Frankenstein

Reputation: 51

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

Upvotes: 1

Sathish
Sathish

Reputation: 3870

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;
}

Upvotes: 6

Alexandru Cimpanu
Alexandru Cimpanu

Reputation: 1069

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

Upvotes: 3

Related Questions