2

I am new to C so I started playing around with some code. I have a bug in my code as I believe using printf(pass) is not safe to use because it can overwrite the value and hence not safe for the user. I was wondering am I right about it that printf(pass) is not safe in my code? Also, how can I still let user print the message finally logged in without changing my code. Is there any way to do that?

My code:

#include <stdio.h>

char pass[100];

char getPass() {
  int value = 'G';
  int * j = & value;
  fgets(pass, sizeof(pass), stdin);
  printf("your entered pass is ");
  printf(pass);
  return (char)( * j);
}

void main() {
  printf("enter the pass here ");
  if (getPass() == 'K') {
    printf("finally logged in\n");
    exit(0);
  } else {
    printf("Wrong password\n");
    exit(1);
  }
}
Jabberwocky
  • 48,281
  • 17
  • 65
  • 115
  • *"it can overwrite the value"* - what value? What are you worried about overwriting? `printf` only writes to `stdout` so it's unclear what is unsafe here – UnholySheep Sep 24 '20 at 08:43
  • 2
    You mention `print` several times. Do you mean `printf`? Also, your code would be a *lot* easier to read if you applied some standard indenting to it. – Tom Karzes Sep 24 '20 at 08:45
  • @UnholySheep I meant if it overwrite whatever user types as in password. The user would be able to log in with the wrong password as well. –  Sep 24 '20 at 08:49
  • @TomKarzes Sorry about that I have indented my code. And, yes I meant printf. I mean is it buffer overflow vulnerability? –  Sep 24 '20 at 08:50
  • `printf` should not overflow its internal buffer. It will flush the output before that happens. – Tom Karzes Sep 24 '20 at 08:51
  • Then what is buffer overflow vulnerability in my code? @TomKarzes –  Sep 24 '20 at 08:53
  • Instead of using `printf(pass)`, you need to do `printf("%s\n", pass)`, or possibly `puts(pass)`. Otherwise it will not behave correctly if the string contains a `%` character. `pass` is a string, not a format string. By passing it as the first argument to `printf`, you're using it as a format string. – Tom Karzes Sep 24 '20 at 08:56
  • Side comment: there is another vulnerability. Usually you don't want to print passwords on the terminal. See https://stackoverflow.com/questions/6856635/hide-password-input-on-terminal –  Sep 24 '20 at 08:57
  • So, did you mean if a user types % while entering the password it would not behave correctly? @TomKarzes –  Sep 24 '20 at 08:58
  • @Jean-ClaudeArbaut I see. Is there any way to print the "finally logged in" message or figure out the password somehow without making any chances in my code? –  Sep 24 '20 at 08:59
  • 2
    @Sky Right. It should be obvious. What would happen if the password were `%s%d%p%x`? What do you think `printf` would do with a format string like that? It would look for additional arguments that aren't there, so it might seg fault, or it might pick up some garbage values and then expand them. One thing it would *not* do is print the password. – Tom Karzes Sep 24 '20 at 09:01
  • @TomKarzes Can you please give me an actual example so I can print the message "finally logge in" in my console? –  Sep 24 '20 at 09:04

2 Answers2

6

Yes it's unsafe, but not for the reason you suggested. If you check the man page for the printf() function, you'll see it has the following prototype:

int printf(const char * restrict format, ...);

The const modifier applied to the first argument specifies that the value of this parameter will not be changed when you call this function.

However, you'll also notice that this parameter is called format. That's because it's supposed to specify a format string. In your program, there's nothing to stop a user entering a format specifier like %p, in which case your call to printf() will start printing out the contents of the stack. There's a Wikipedia article that describes this vulnerability in more detail.

r3mainer
  • 23,981
  • 3
  • 51
  • 88
  • I understand it. So is it the only vulnerability in my code? Also, how can I still make user print "finally logged in" message somehow. Is it possible? –  Sep 24 '20 at 08:51
  • 1
    `printf("finally logged in\n");` would be better coded as `puts("finally logged in");` but isn't vulnerable. The problem is really with `printf(pass);`, where `pass` is obtained from user input. Use `printf("%s", pass);` instead. – r3mainer Sep 24 '20 at 09:06
  • I see. But, I mean is there any way to print `finally logged in` without changing my code as in not changed it to `puts()` or anything and use that vulnerabilty to output somehow `finally logged in` message as it will only output that if it is equal to `k` –  Sep 24 '20 at 09:09
  • Yes, there is. Read the documentation for `printf()`. The `%n` specifier stores the number of characters printed so far at a location specified by a pointer. If a user entered a string of 75 characters followed by `%n`, then your call to `printf()` would store the value 75 (the ASCII code for `K`) at the location of a pointer on the stack. With a little effort, this could be made to overwrite the value of `int value`. (Hint: try doing something with the pointer `j`.) – r3mainer Sep 24 '20 at 09:39
  • I try writing K after 75 charcters and kept doing it but it didn't work. For example, I wrote K at 76th place, 77th and so on, but it didn't overwrite the value. Could you please help? I have been trying to figure it out to what you said and even read the documentation I am still stuck since past hours –  Sep 24 '20 at 10:46
  • You need to understand how the stack is organised. There really isn't enough space to explain here. Start by reading this: [Smashing The Stack For Fun And Profit](https://inst.eecs.berkeley.edu/~cs161/fa08/papers/stack_smashing.pdf). – r3mainer Sep 24 '20 at 10:55
  • Can you move this conversation to chat then? If that's easier to explain and yes, I will read that now. Thanks. –  Sep 24 '20 at 11:21
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/222004/discussion-between-r3mainer-and-sky). – r3mainer Sep 24 '20 at 11:44
4

The function fgets if its call was successful provides a string that you can output.

A problem can arise if the obtained string is outputted such a way

printf(pass);

and contains conversion specifiers.

So instead of that call

printf(pass);

it is a more safer to use

printf( "%s", pass );

or

puts(pass);

Preliminary you can remove the new line character stored in the string by the call of fgets.

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
  • I was wondering is it the buffer overflow vulnerability (printf(pass) here? That's what I meant by unsafe. –  Sep 24 '20 at 08:49
  • @Sky As you explicitly specified the size of the array in the call of fgets sizeof(pass) then there is no buffer overflow. – Vlad from Moscow Sep 24 '20 at 08:50
  • Also, how can I make user print "finally logged in"? Is it possible even when the password is not right ot somehow overwriting something? –  Sep 24 '20 at 08:52
  • Did you mean there is no buffer overflow even in printf(pass) as well? –  Sep 24 '20 at 08:52
  • @Sky There is no buffer overflow. Though there can be undefined behavior if the string contains conversion specifiers. – Vlad from Moscow Sep 24 '20 at 08:54
  • Also, could you you explain a bit more why is it not safe to use that? –  Sep 24 '20 at 08:55
  • Ah, thanks. Also, my another part of question was to still print the "finally logged in" message without changing my code, so somehow figure out the password If you could explain that? I will accept the answer then –  Sep 24 '20 at 08:55
  • @Sky If for example the entered string is "%s". In this case the compiler will use an arbitrary address to output a string that should correspond to the conversion specifier %s. – Vlad from Moscow Sep 24 '20 at 08:56
  • I see. Could you explain another part of my question as well? –  Sep 24 '20 at 09:01
  • @Sky What is the another part? – Vlad from Moscow Sep 24 '20 at 09:01
  • How can I print the "finally logged in" message. As right now I am not sure how to print that. As if there is a vulnerabilty I wanna use that to maybe somehow get the password as `K` by overwriting so it will print that statement. As it will only print if getpass is equal to `k` –  Sep 24 '20 at 09:03
  • @Sky I am sorry I have not understood what you mean. – Vlad from Moscow Sep 24 '20 at 09:06
  • I mean is there any way if you run this code in some IDE and make it print the statement `finally logged in`? Does it makes sense? Let me know please –  Sep 24 '20 at 09:07
  • @Sky In general the call of printf that can invoke undefined behavior can overwrite the memory occupied by the object j of the type char. – Vlad from Moscow Sep 24 '20 at 09:19
  • Can you show any example or figure out the password to print that statement? –  Sep 24 '20 at 09:23
  • @Sky As I said this can be a result of undefined behavior. Try to use as the entered string a string like "%s" or "%.100s" and similar. – Vlad from Moscow Sep 24 '20 at 09:29
  • I see. I am trying to figure out the password. Shall I try typing more than 100 charcters and add K at the end and see when it works? Like if I write any 100 characters and K at 101 then K at 102 place would it somehow overwrite it? I am try to print that message somehow if you could please run his program and help me? –  Sep 24 '20 at 09:33
  • @Sky The probability is very low. You need to see the generated assembler code. – Vlad from Moscow Sep 24 '20 at 09:34
  • So did you mean I can't print that line? –  Sep 24 '20 at 09:36
  • @Sky I mean that I can not say whether you can or not can because there can be undefined behavior. – Vlad from Moscow Sep 24 '20 at 09:42