C++ Buffer Overflow, strcpy, fgets, sprintf showing runtime error

Question

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
int check_authentication(char* password) {
    int auth_flag = 0;
    char* password_buffer;
    char* dept;
    password_buffer = (char*)malloc(16);
    dept = (char*)malloc(10);
    printf("Your department?");
    fgets(dept, 10, stdin); //line 11
    strcpy_s(password_buffer, 16, password); //line 12
    if (strcmp(password_buffer, "AsiaPacificInst") == 0) {
        if (strcmp(dept, "NSF") == 0) {
            auth_flag = 1;
        }
    }
    if (strcmp(password_buffer, "AsiaPacificUni") == 0) {
        if (strcmp(dept, "TM") == 0) {
            auth_flag = 1;
        }
    }
    return auth_flag;
}
int main(int argc, char* argv[]) {
    char errmsg[512];
    char outbuf[512];
    char user[20];


    printf("Username: ");
    fgets(user, 20, stdin); //line 32
    if (strcmp(user, "Adm1n") == 0) {
        printf("Authorised User\n"); sprintf_s(errmsg, "Authorised User %400s", user); sprintf_s(outbuf, errmsg);  //line 34

        if (argc < 2)
        {
            printf("Usage: %s <password>\n", argv[0]); exit(0);
        }

        if (check_authentication(argv[1]))
        {
            printf("\n-=-=-=-=-=-=-=-=-=-=-=-=-=-\n");
            printf(" Access Granted.\n");
            printf("-=-=-=-=-=-=-=-=-=-=-=-=-=-\n");
        }

        else {
            printf("\n-=-=-=-=-=-=-=-=-=-=-=-=-=-\n");
            printf("\nAccess Denied.\n");
            printf("\n-=-=-=-=-=-=-=-=-=-=-=-=-=-\n");
        }


    }
    else { printf("Unauthorised User!!\n"); exit(0); }

}

I need help on to check whether the set of codes below has been written in the correct way as I'm not familiar with C++.

fgets (line 11 and 32)
strcpy_s (line 12)
sprintf_s (line 34)

Because these line of codes had errors earlier when I got them from another source. However, I fixed those error yet on runtime the program did not work properly. The program actually should request for a username and password and verify whether the user is authorized using the username and verify the user's department using their password. However, I could only enter the username when I run the program. It did not request me a password. Also in overall is there any other issue that may cause the program to not run properly.

Program Result when executed

https://stackoverflow.com/questions/2693776/removing-trailing-newline-character-from-fgets-input — Retired Ninja, Aug 09 '21 at 07:56
That's pure C. While it should be compilable as C++, most C++ programmers would not call it "C++". — molbdnilo, Aug 09 '21 at 07:59
`fgets` reads the newline from the buffer, so `dept` can never be `"NSF"`, but `"NSF\n"`. — mch, Aug 09 '21 at 08:02
Some thoughts: you seem to mix up authentication and authorization in your code. These are two entirely different concepts that you'll want to separate. Also, authentication code deals with untrusted input by default. You'll want to make sure (read "unit tests" if not "proof") that you understand the code and it actually works even if an attacker dumps an AVI into any of your buffers. You'll also want to DRY and create constants for your buffer lengths to be reused with your fgets. — xmjx, Aug 09 '21 at 08:04
`password_buffer = (char*)malloc(16);` makes no sense, simply declare `char password_buffer[16];` and save the allocation (or better `std::string password_buffer{};`) (the same for `dept`). Neither are returned, so there is no need for the memory to survive beyond the function call (which leaks memory because neither are freed). — David C. Rankin, Aug 09 '21 at 08:11
@KhohulaRhaj It will compile with a `C++` compiler, but it's not. The headers you are using are all `C` ('stdlib.h' instead of `cstdlib`, and similar). You also don't use anything that is not standard `C` — Lala5th, Aug 09 '21 at 08:11

Rustam Hovhannisyan · Accepted Answer · 2021-08-09T08:19:29.257

The program doesn't request password, because it expects it to be passed as an argument like this: 'c:\yourapp.exe yourpass'. If you want it to request a password, you should modify it a bit.

Add following lines before if (check_authentication(argv[1])) line in your main function.

char password[16];
printf("Password: ");
fgets(password, 16, stdin);

Replace line if (check_authentication(argv[1])) with if (check_authentication(password))

And delete or comment out following lines:

if (argc < 2)
{
    printf("Usage: %s <password>\n", argv[0]); exit(0);
}

And finally please do not forget removing newlines after each fgets call. Removing trailing newline character from fgets() input

C++ Buffer Overflow, strcpy, fgets, sprintf showing runtime error

1 Answers1