tags:

views:

225

answers:

7

hi - I am going through a book and I tried running this example but I receive a segmentation fault - gdb says it's when it sets argv[0] = filename;

this code is copied/pasted straight from book's downloadable code samples.

#include <unistd.h>

int main() {
  char filename[] = "/bin/sh\x00";
  char **argv, **envp; // arrays that contain char pointers

  argv[0] = filename; // only argument is filename - segmentation fault here
  argv[1] = 0;  // null terminate the argument array

  envp[0] = 0; // null terminate the environment array

  execve(filename, argv, envp);
}

Edit: The book is Hacking: The Art of Exploitation by Jon Erickson, which has VERY good reviews. This specific example is used as the first tutorial on converting C into machine code in the shellcode section, specifically it is exec_shell.c and can be downloaded from http://nostarch.com/hacking2/htm . I imagine some context around the use of this code was necessary in order to avoid some of the negative comments below, sorry for leaving details out, and thanks for the help.

+12  A: 

It obviously isn’t a very good book. The problem is that neither argv nor envp are initialized, so when you write to argv[0], you’re trying to overwrite some random location in memory.

Try something like this:

#include <unistd.h>

int main() {
  char *filename = "/bin/sh";
  char *argv[2], *envp[1];

  argv[0] = filename;
  argv[1] = 0;

  envp[0] = 0;

  execve(filename, argv, envp);
}

This alternative initializes argv and envp on the stack, with enough space to contain two pointers and one pointer respectively.

In the code above, I’ve made one additional change to repair an additional common (but, in this case, harmless) misunderstanding. The \x00 that was at the end of "/bin/sh\x00" is redundant, since in C static strings are implicitly null-terminated. "/bin/sh\x00" is a string terminated by two nulls.

Alternatively, as pointed out by caf, here is a more compact example with exactly equivalent meaning:

#include <unistd.h>

int main() {
  char *filename = "/bin/sh";
  char *argv[2] = { filename, 0 };
  char *envp[1] = { 0 };

  execve(filename, argv, envp);
}
Daniel Cassidy
Kedar Soparkar
The string `"/bin/sh/\x00"` is allocated statically. `argv` doesn’t have any other contents. `envp` doesn’t have any contents at all.
Daniel Cassidy
In fact you might as well just have `char *argv[] = { "/bin/sh", 0 }, *envp[] = { 0 };`
caf
+2  A: 

char **argv

argv is pointing to a memory location which you are not allowed to access/write to. It is something that is better known as a wild pointer.

Prasoon Saurav
+3  A: 

You never allocate the "arrays of pointers" meant to go in argv and envp! What book is it, that omits such crucial steps?!

Either add argv = malloc(2 * sizeof(char*)) (and similarly for envp) before you start assigning to argv[0] and friends, or change argv's and envp's declarations to be arrays of pointers rather than pointers to pointers (the latter's quite a feasible approach, in this specific case, since you do know exactly how many pointers you need in each at the time you're writing the code -- dynamic allocation is therefore somewhat supererogatory;-).

Alex Martelli
Should be allocating 2*sizeof(char*)
Andrew Medico
@Andrew, entirely correct, tx and +1 for spotting my typo (I've edited the A to fix it).
Alex Martelli
A: 

Looks like you need to get a better book! In this code argv is a pointer with no storage allocated to it and pointing at random memory (or probably NULL). When you dereference it with argv[0] = ..., your code ends up trying to write to random memory. Your variable declaration should be something more like:

char *argv[3], *envp[1];
llasram
It's probably not `NULL`. I don't know of any operating system or runtime that normally zeros out memory on launch, and no implicit initialization is done for automatic variables.
Chuck
A: 

I have no idea where did you get this book, but it obviously sucks. argv is an uninitialized pointer, it holds a random address. Hence accessing it will most probably lead to the access violation.

valdo
A: 

Before using such multi-level pointers, I recommend reading up on dynamic memmory allocation in C.

Whenever you use pointers, you must also think whether you need to allocate space for the data that the pointers are going to point to (as also the pointers themselves, for multi-level pointers).

For example,

char **bar; 

here, bar allocates space for 1 pointer-to-pointer, ie. enough space to store one address. This is not very useful without any additional data allocation.

In reality, you should be doing:

char **bar = calloc( 2 , sizeof(char *) );

here, bar allocates space for 1 pointer-to-pointer, ie. again, space to store one address as bar, AND 2 consecutive locations for storing 2 more pointers, namely bar[0] & bar1.

char bar[0]= calloc( 10 , sizeof(char) );

here, bar[0] allocates space for storing a string of size 10 - 1 (for \0 at end).

Now, if you do a string copy:

strcpy(bar[0],"Hello!");

the final memory map comes to: (addresses in circles, contents in blocks) alt text

Kedar Soparkar
A: 

Many of the people here are on the right track, but missing some of the numerous problems here.

#include <unistd.h>

int main() {
  char filename[] = "/bin/sh\x00";
  char **argv, **envp; // arrays that contain char pointers

  argv[0] = filename; // only argument is filename - segmentation fault here
  argv[1] = 0;  // null terminate the argument array

  envp[0] = 0; // null terminate the environment array

  execve(filename, argv, envp);
}

The problems here are:
1. The pointer array of character strings is never initialized. Pointers take up space too, and thus an array of pointers needs to use malloc in c.
2. Each character pointer in your pointer array needs its own malloc statement before use.

Here is the working code, with printouts to show you what is going on:

#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main() {
  unsigned int i=0;
  char filename[] = "/bin/sh\x00";
  char **argv; // arrays that contain char pointers

  argv=(char **)malloc(sizeof(char*));
  argv[0]=(char *)malloc(strlen(filename)*sizeof(char));
  strcpy(argv[0],filename);

  printf("Arg 0 is %u chars long...\n",strlen(argv[0]));
  printf("Arg 0 is ");
  while (argv[0][i] != '\0') {
    printf("%c",argv[0][i]);
    i++;
  }
  printf("!\n");

  free(argv[0]);
}
Jason R. Mick
Err. No. In this case you **can** directly assign one character array to another. It’s an area to be careful around, certainly, and if in doubt it’s best to make a copy. But, in this case we know that `execve` won’t modify the strings (its arguments are defined like `char *const argv[]`). We also know that the strings won’t get used after the function returns (because `execve` never returns). So, there’s no reason to waste time and effort copying the strings. Just give `execve` pointers to the original strings and be done.
Daniel Cassidy
Also, if your code did call `execve`, it wouldn’t work, because `execve` requires that the `argv` and `envp` arrays are terminated by a null pointer, and you only made room for one pointer in `argv`.
Daniel Cassidy
Point taken. They're just assigning the char pointer to the previously allocated character array, so that is acceptable. Still better to perform a malloc/strcpy to create a fresh copy, just to be safe and prevent accidental modification in longer programs. Just be sure to clean up your new memory with free at the end of your program (as I did).
Jason R. Mick