Tweaked getpass() function gives an untraceable bug

Applications Programming
1

I have customized the getpass() as follows:

char* my_getpass(const char* str) {
    struct termios oflags, nflags;
    static char passwd[64];

    /* disabling echo */
    tcgetattr(fileno(stdin), &oflags);
    nflags = oflags;
    nflags.c_lflag &= ~ECHO;
    nflags.c_lflag |= ECHONL;
    tcsetattr(fileno(stdin), TCSANOW, &nflags);
    printf(str);
    fgets(passwd, sizeof(passwd), stdin);
    passwd[strlen(passwd) - 1] = '\0';

    /* restore terminal */
    tcsetattr(fileno(stdin), TCSANOW, &oflags);
    return passwd;
}

int main() {
    char name[8], *pwd;
    printf("Login as: ");
    fgets(name, sizeof(name), stdin);
    pwd = my_getpass("Password: ");
    if (strcmp(pwd, "george"))
        fprintf(stderr, "\nPassword incorrect\n");
    else {
        puts("\nCorrect Password");
        FILE* fp = fopen("./passwd.txt", "w");
        fprintf(fp, "user: (%s), password: (%s) \n", name, pwd);
        fclose(fp);
    }
}

but now the output file "password.txt" contains a carriage return in the result as

$ more passwd.txt 
user: (dude
), password: (george)

which should have been as,

user: (dude), password: (george)

could anyone suggest me how to correct this bug? :frowning:

2 
    fgets(passwd, sizeof(passwd), stdin);
    passwd[strlen(passwd) - 1] = '\0';

This looks to me like you're trying to add a null-terminator to the string.

strlen() only works when the string already has a null terminator.

fgets() gives it a null terminator anyway.

It's not a bug, it's doing exactly what you tell it to... fgets() adds the return key to the string. So change it to this:

    fgets(passwd, sizeof(passwd), stdin);
    passwd[strlen(passwd) - 2] = '\0';

...to wipe out the very last character in the string, the newline.

Also: Make your buffer 4096, not 8, or someone is almost guaranteed to overflow and crash it.

1 Like
3

Thanks a lot for the impeccable and intuitive hint!! hats off to you "The Geek" :slight_smile:

But a small correction is, I have to do it for the name variable inside main() instead of passwd variable in the my_getpass() function like

    printf("Login as: ");
    fgets(name, sizeof(name), stdin);
    name[strlen(name) - 1] = '\0'; // newly added line for the bug

it then works fine and gives me the expected output.

4

I suspect you'll want to do both, actually, since there will be a \n on the end in both places.

This is a common enough need I usually make it a function:

void chomp(char *str)
{
  int len=strlen(str);

  if(len>0)
  while((len>0) && isspace(str[len-1]))
    str[--len]='\0';
}

then you just

fgets(buf, size, stdin);
chomp(buf);
1 Like