views:

657

answers:

4

Hi, I know it's possible to overflow ordinary code:

char string[9];

scanf("%s", string).

But is it possible to overflow scanf("%8s", string)? 8 is just an example.

I know "%8s" works like a delimit, but I also notice when I input string longer than 8 chars, the program will terminate due to:

* stack smashing detected *: ./a.out terminated

======= Backtrace: =========

...

Obviously there's a flag that detects stack smashing turned on by GCC by default. Since this is a stack smashing, then my guess is that it is still possible to overflow and execute arbitrary code.

Contrary to normal overflow that mangles the caller of scanf("%s"), if scanf("%8s") can overflow, it will overflow within scanf function so that when scanf try to return, control is gained.

But scanf is a syscall that requires mode-switch (switching from user mode into kernel mode), and internally it will call stuff like read to the stdin etc. So not sure if we can overflow in kernel mode or something..

Comments are welcome!!

UPDATE >>

char string[9] is assumed in the above example. char string[8] in following real code.

The question is really about the seeming conflicting story between safe scanf("%8s") and GCC abortion due to stack smashing.

Simplified code:

void foo(pass some pointer) {
char input[8];
int input_number = 0;

while (1) { // looping console
   printf some info;
   scanf("%8s", input);

   input_number = atoi(input);

   if ((strlen(input) == 1) && (strncmp(input, "q", 1) == 0)) {
       input_number = -1;
   }
   switch (input_number) {
       case -1: to quit the console if input = 'q';
       default: to print info that pointer refers to;
       ...
   } 

}

}

Note:

  1. foo is called by someone else.
  2. Though string is 8 bytes in real code with "%8s", I don't think this lead to smashing.
+1  A: 

if string is allocated for less then 8 charters it will certainly overwrite the buffer also scanf will not append a null terminator. But as long as you have enough space in string for your value you should not get an overwright.

rerun
It would need at least 9 bytes to not overflow.
wallyk
Actually I think scanf will put a '\0' at the end. C standard says "a terminating null character, which will be added automatically.", also quoted by paxdiablo
Figo
I meant if you didn't also have a space for the null.
rerun
If you don't have a space for the null, it will put it there regardless (overwriting some other part of the stack if neccessary).
paxdiablo
char string[9] should be alright.
Figo
+2  A: 

Don't ever use scanf (or fscanf for that matter) if you want your input to be robust.

You should be using fgets (or a similarly "protected from buffer overflow" variant) then use sscanf on that.

The main problem with scanf and fscanf is that your file pointer can end up in an indeterminate position if the line is not of the expected format (i.e., if the scanf fails). With the fgets/sscanf method, it's a lot easier to guarantee that you're on a line boundary, without having to use ftell and fseek to move around the file.

Regarding your specific query about whether the buffer will overflow, the C standard has this to say:

... the corresponding argument shall be a pointer to the initial element of a character array large enough to accept the sequence and a terminating null character, which will be added automatically.

So, for a "%8s" format, you need a 9-character array.

I suspect you have some other problem in your code. With a test program:

#include <stdio.h>
int main(int argc, char* argv[]) {
    char x1;
    char a[9];
    char x2;
    x1 = x2 = ' ';
    scanf ("%s",a);
    printf ("[%c] [%s] [%c]\n",x1,a,x2);
    return 0;
}

I get:

pax> ./qq.exe
dfjdhadgha...lghjdfgjhd
[s] [dfjdhadgha...lghjdfgjhd] [ ]
  6 [main] qq 4744 _cygtls::handle_exceptions: Error while dumping state
  (probably corrupted stack)
  Segmentation fault (core dumped)

When I change that same program to use "%8s", I get (for exactly the same input):

pax> ./qq.exe
dfjdhadgha...lghjdfgjhd
[ ] [dfjdhadg] [ ]
paxdiablo
Yeah, I know. But right now I'm interested in knowing if scanf(%8s) has the same problem as scanf, as GCC is telling me there's still a stack smashing happening!
Figo
Describe "robust"? Give some examples?
ysth
@ysth: (1) Get your input as lines. (2) Ensure you get whole lines (\n char at end), error otherwise with "line too long". (3) Use sscanf on line - you can do this as many times as you want on the line without worrying about the underlying file.
paxdiablo
I also test a similar simplied code like you did and GCC won't complain about stack smashing..I have posted orig code and the thing is at run time there will be GCC complain when I input 12345678 (Note in real code char string[8]).
Figo
@Figo, you need a 9-character array to store an 8-character string. That's because an 8-character string consist of the 8 characters (obviously) and a null terminator character (not so obvious). If you change it to char string[9], you shouldn't have any problems.
paxdiablo
+5  A: 

See http://www.opengroup.org/onlinepubs/009695399/functions/scanf.html:

Each directive is composed of one of the following...An optional non-zero decimal integer that specifies the maximum field width.

s
Matches a sequence of bytes that are not white-space characters. The application shall ensure that the corresponding argument is a pointer to the initial byte of an array of char, signed char, or unsigned char large enough to accept the sequence and a terminating null character code, which shall be added automatically.

So it won't overflow a 9-byte string buffer.

ysth
If it won't overflow, why does GCC telling the Stack Smashing story still?
Figo
@Figo: because you've made some mistake. Show your code!
ysth
@ysth: code posted.
Figo
@Figo: somehow you are not understanding. "%8s" will store up to 9 bytes, so you need a nine character array.
ysth
+1  A: 

As ysth pointed out, the array should be able to contain the string and the terminating null-character, so using an 8-byte array (especially if it's allocated on the stack, as it is in your code) is very likely to mess it up.

Tal Pressman