0

I was trying to get user name and password for authentication on a linux system and gcc-4.7 compiler as follows:

#include "stdio.h"
#include "stdlib.h"
#include <termios.h>
#include <unistd.h>

void getuserdata(char *passwd)
{
    struct termios term, term_orig;
    tcgetattr(STDIN_FILENO, &term);
    term_orig = term;
    term.c_lflag &= ~ECHO;
    tcsetattr(STDIN_FILENO, TCSANOW, &term);

    scanf("%s", passwd);
    /* Remember to set back, or your commands won't echo! */
    tcsetattr(STDIN_FILENO, TCSANOW, &term_orig);

}

int main()
{
    char *password, *username;
    printf("Enter username: ");
        scanf("%s", username);
        fflush(stdin); 
    printf("\n");

    printf("\nEnter password: ");
    getuserdata(password);

    printf("Entered username is:%s\n", username);
    printf("Entered password is:%s\n", password);
    return 0;
}

and expected behaviour was as follows:

Enter username: test 
Enter password:
Entered username is: test 
Entered password is: xxxx

But it is not working and giving null as username or password.

Guys where am I going wrong?

Any help would be greatly appreciated.

Thanks.

Haris
  • 12,120
  • 6
  • 43
  • 70
Abhijatya Singh
  • 642
  • 6
  • 23
  • 2
    `char *password, *username;` are uninitialized pointers that point nowhere. You need to either allocate memory to store your input in or declare them as static character arrays of sufficient size to hold your data (e.g. `char password[32] = {0}, username[48] = {0};`)... – David C. Rankin Feb 02 '16 at 04:42
  • Be wary of [Using `fflush(stdin)`](http://stackoverflow.com/questions/2979209/using-fflushstdin). It works on Windows using the MS support libraries; it doesn't necessarily work when using GNU libraries on WIndows, and it doesn't work usefully on most other platforms. – Jonathan Leffler Feb 02 '16 at 04:47
  • this line: `term_orig = term;` will not copy the struct thermios structure. What is actually needed is: `memcpy( &term_orig, &term, sizeof( struct termios) );` – user3629249 Feb 03 '16 at 01:49
  • regarding lines like: `scanf("%s", username);` the format specifier '%s' does not limit the number of characters that the user can input. Therefore the user can overflow the input buffer. Given the other problem of the username and password pointers contain trash rather than pointing to allocated memory, This problem (and the reference problem,) are undefined behaviour and can lead to a seg fault event. suggest: `scanf("%###s", username);` where ### is the allocated memory size -1 (Need the -1 because scanf() with the'%s' format specifier will always append a NUL byte to the input buffer – user3629249 Feb 03 '16 at 01:54
  • The standard says `fflush()` is only for output streams and says using it on an input stream is undefined behaviour. Suggest: replace with: `int ch; while( (ch = getchar() ) != EOF && '\n' != ch );` – user3629249 Feb 03 '16 at 01:57

1 Answers1

3

You did not allocate memory for username and password.

char *password, *username;
username = malloc(50);
printf("Enter username: ");
    scanf("%49s", username);

printf("\n");

printf("\nEnter password: ");
password = malloc(50);
getuserdata(password);
Haris
  • 12,120
  • 6
  • 43
  • 70
  • 1
    @AbhijatyaSingh: accidents will happen, and coincidences. When you don't allocate the space, the system can do what it likes, which can include seeming to work some of the time. You're using _undefined behaviour_ which is unconditionally a bad idea. You're also mixing `stdin` operations with operations on `STDIN_FILENO`; that's problematic. You may get away with it, but be careful. – Jonathan Leffler Feb 02 '16 at 04:50
  • Can't we allocate memory like this: `username = (char *) malloc(sizeof(char));` ? – Sagar Patel Feb 02 '16 at 04:55
  • @SagarPatel, `sizeof(char)` is 1 byte. And I am sure you would want to store a username bigger then 1 character. ANd you do not need to typecast the return of `malloc()`. – Haris Feb 02 '16 at 04:57
  • @Haris, But i tried allocating memory like `username = (char *) malloc(sizeof(char));` this and it works fine. – Sagar Patel Feb 02 '16 at 04:59
  • @SagarPatel, It will work fine if you enter a `username` of 1 character. As you have only 1 Byte allocated. If you enter and store more then 1 Byte, it will lead to Undefined behaviour. It might work this time, but will surely not work all the time. – Haris Feb 02 '16 at 05:07
  • Thanks everybody for the help and suggestion it really helped alot. Thanks again..... – Abhijatya Singh Feb 02 '16 at 06:04
  • That last line should be `getuserdata(password);` without the `&`. – dxiv Feb 02 '16 at 07:22
  • @dxiv, Thanx. But he changed the code. Earlier it was different. – Haris Feb 02 '16 at 07:31
  • `fflush(stdin)` causes undefined behaviour. Alsoyou should limit the length of scanf input. – M.M Feb 02 '16 at 07:34