New to C code don't fully understand what's wrong

Question

The code I'm using should only give access to password byte however it gives it to bytebyte as well. I changed it from the original gets to fgets which solved a stacksmashing issue. What do I need to do to fix the new issue of bytebyte being excepted.

#include <stdio.h> 
#include <string.h> 
#include <stdlib.h> 
  
int main(void) {
// Use a struct to force local variable memory ordering
  struct {
    char buff[5];
    char pass;
  } localinfo;
  localinfo.pass = 0;
  
  printf("\n Enter the password:\n");
  fgets(localinfo.buff, 5, stdin); // Get the password from the user
  // Check the password with string matching
  if(strcmp(localinfo.buff, "byte") !=0 ){
    printf ("\n Wrong Password \n");
  }
  else {
    printf ("\n Correct Password\n");
  localinfo.pass = 1; // Set a flag denoting correct password
  }
//IF password matches
// GIVE root or admin rights to user by checking for flag
  if(localinfo.pass){ 
    printf ("\n Congratulations! Root privileges given to the user!\n");
  }
  return 0;
}

How do you account for the `'\n'` read and included by `fgets()` in `strcmp(localinfo.buff, "byte")`?? (hint: `localinfo.buff[strcspn (localinfo.buff, "\n")] = 0;`) — David C. Rankin, Sep 22 '22 at 00:51

score 2 · Answer 1 · answered Sep 22 '22 at 00:27

fgets() will only read as many bytes as you specify in the size argument, minus 1 for the null terminator. So fgets(localinfo.buff, 5, stdin); will only read the first 4 bytes of the input. If the user enters bytebyte, only byte will be read into the string, and the comparison will succeed.

You should read the input into a buffer longer than any password you want to compare with. Only copy to localinfo.buff once you've determined that the password is valid.

Also, don't forget that fgets() leaves the newline in the input if it fits; you should remove this before using the input. See Removing trailing newline character from fgets() input

Andreas Wenzel · Accepted Answer · 2022-09-22T01:50:53.383

You declared localinfo.buff like this:

char buff[5];

This means that the array only has room for 4 characters plus the terminating null character.

With the input bytebyte, the line

fgets(localinfo.buff, 5, stdin);

will only read the first 4 characters into localinfo.buff and leave all other characters on the input stream. This is not what you want.

I suggest that you use an input buffer with a generous size of maybe size 200, and verify that the entire line was read in by verifying that the newline character was found. Afterwards, you should remove the newline character, because if the input contains a newline character, it will never match the desired password.

Here is an example:

#include <stdio.h> 
#include <stdlib.h> 
#include <string.h> 
  
int main(void)
{
    char line[200], *p;

    //prompt user for input  
    printf("Enter the password: ");

    //attempt to read one line of user input
    if ( fgets( line, sizeof line, stdin ) == NULL )
    {
        printf( "Input error!\n" );
        exit( EXIT_FAILURE );
    }

    //attempt to find newline character
    p = strchr( line, '\n' );

    //verify that entire line was read in
    if ( p == NULL )
    {
        printf( "Line too long for input buffer!\n" );
        exit( EXIT_FAILURE );
    }

    //remove newline character by overwriting it with null character
    *p = '\0';

    //check the password
    if( strcmp( line, "byte" ) != 0 )
    {
        printf( "Wrong Password.\n" );
        exit( EXIT_FAILURE );
    }

    //print success message
    printf( "Correct Password.\n" );
    printf( "Congratulations! Root privileges given to the user!\n" );

    return 0;
}

This program has the following behavior:

Enter the password: byte
Correct Password.
Congratulations! Root privileges given to the user!

Enter the password: bytebyte
Wrong Password.

Kudos on the most important rule "*Don't Skimp on Buffer Size!*" `:)` — David C. Rankin, Sep 22 '22 at 00:52
@DavidC.Rankin: However, the second rule is: Don't make the buffer so large that it could cause a [stack overflow](https://en.wikipedia.org/wiki/Stack_overflow). :-) As long as it's only a single buffer, a buffer size up to 500 KB should be no problem (except maybe on embedded platforms), but if you have multiple buffers in a deep call tree, buffers of even a few kilobytes could be a problem. — Andreas Wenzel, Sep 22 '22 at 00:58

New to C code don't fully understand what's wrong

2 Answers2