strange behaviour

Discussion in 'C Programming' started by tfelb, Jan 13, 2009.

  1. tfelb

    tfelb Guest

    This function creates a puzzle for me which I don't understand.

    Through the process of s2, the pointer increments beyond the limit of
    the given pointer array in main.
    I have another function where I used a pointer array as an argument
    and a function parameter as a double pointer and this works!?

    s2 must find the NULL after the second element of that pointer array
    but it doesn't.

    Sorry it's maybe a fool question.


    Thanks for any help!

    Tom F.



    /*
    * TEST Function
    */

    void test(char *buffer, char *s1, char **s2)
    {




    while(*s2)
    {

    s2++; /* Steps beyond into garbage */
    }




    }

    /* in main */

    char ja[] = "This";
    char jo[] = "That";
    char je[] = "Nothing";


    char *text[2];

    text[0] = ja;
    text[1] = jo;
    text[2] = je;


    join(erg,"<",text);
     
    tfelb, Jan 13, 2009
    #1
    1. Advertising

  2. tfelb <> writes:

    > This function creates a puzzle for me which I don't understand.

    <snip>
    > void test(char *buffer, char *s1, char **s2)
    > {
    > while(*s2)
    > {
    > s2++; /* Steps beyond into garbage */
    > }
    > }


    This loop (which I assume is sketch since it does no real work) must
    be passed an array of char *s that, eventually, includes a null
    pointer. Without one, the loop goes on eventually accessing pointers
    that outside the array you pass. That is undefined behaviour.

    > /* in main */
    >
    > char ja[] = "This";
    > char jo[] = "That";
    > char je[] = "Nothing";
    >
    > char *text[2];


    2 is not even big enough for the three pointers you assign to as the
    elements of text! Change the 2 to 4 and add...

    > text[0] = ja;
    > text[1] = jo;
    > text[2] = je;


    text[3] = NULL;

    and you will be alright (at least as far as the example loop above is
    concerned).

    > join(erg,"<",text);


    --
    Ben.
     
    Ben Bacarisse, Jan 13, 2009
    #2
    1. Advertising

  3. tfelb wrote:

    > void test(char *buffer, char *s1, char **s2)
    > {
    > while(*s2)
    > {
    >
    > s2++; /* Steps beyond into garbage */
    > }
    > }


    Well, of course it does, if it get's not proper terminator. A
    proper terminator was if *s2 evaluated to 0. So the terminating
    element of the array you give as s2 should be a null pointer.

    > /* in main */
    >
    > char ja[] = "This";
    > char jo[] = "That";
    > char je[] = "Nothing";
    >
    >
    > char *text[2];


    When defining an array the value between the brackets is the
    _count_ of elements, the array holds. So to hold 3 values you
    must write:

    char *text[3];

    > text[0] = ja;
    > text[1] = jo;
    > text[2] = je;


    Here you're indexing. Indexing can be understood as

    first element + index

    So to get the first element you use index 0, for the second
    element index 1...

    > join(erg,"<",text);


    Well, you didn't tell us, how "join" is defines, but the way you
    call it and the way it crashes allows for some /assumptions/
    (not statements, let's make this straight):

    join looks like some function, that (I presume it from the name)
    concatenates a array of strings into a single string, putting
    the string defined in the second argument between the parts.

    To indicate, that the last element has been reached, a NULL
    pointer shall be provided as the terminating (in most cases this
    is the last) array element.

    The function "test" supports this assumptions, as it's while loop
    exhibits the very same termination logic. So to be correct, your
    main-code should be:

    char ja[] = "This";
    char jo[] = "That";
    char je[] = "Nothing";

    char *text[4];

    text[0] = ja;
    text[1] = jo;
    text[2] = je;
    text[3] = NULL; /* NULL == 0 BTW */

    join(erg, "<", text)


    NOTE: The interface of "join" and "test" are both inerhently
    unsafe: They both assume, that the 'buffer' argument points to a
    memory location with enough space allocated to hold the result.

    Even if the functions have a hard coded limit, so that you can't
    overrun the buffer with 'overlong' strings in the 's2' argument,
    it's still possible to give a too short buffer, thus causing a
    overrun. Buffer overruns have been the most common cause for
    security holes, as they allow to inject code into a program
    (google for "buffer overrun exploit techniques").

    Wolfgang Draxinger
    --
    E-Mail address works, Jabber: , ICQ: 134682867
     
    Wolfgang Draxinger, Jan 13, 2009
    #3
  4. tfelb

    tfelb Guest

    On 13 Jan., 14:57, Wolfgang Draxinger <>
    wrote:
    > tfelb wrote:
    > > void test(char *buffer, char *s1, char **s2)
    > > {
    > >      while(*s2)
    > > {

    >
    > > s2++; /* Steps beyond into garbage */
    > > }
    > > }

    >
    > Well, of course it does, if it get's not proper terminator. A
    > proper terminator was if *s2 evaluated to 0. So the terminating
    > element of the array you give as s2 should be a null pointer.
    >
    > > /* in main */

    >
    > > char ja[] = "This";
    > > char jo[] = "That";
    > >                 char je[] = "Nothing";

    >
    > >                 char *text[2];

    >
    > When defining an array the value between the brackets is the
    > _count_ of elements, the array holds. So to hold 3 values you
    > must write:
    >
    > char *text[3];
    >
    > > text[0] = ja;
    > > text[1] = jo;
    > > text[2] = je;

    >
    > Here you're indexing. Indexing can be understood as
    >
    > first element + index
    >
    > So to get the first element you use index 0, for the second
    > element index 1...
    >
    > > join(erg,"<",text);

    >
    > Well, you didn't tell us, how "join" is defines, but the way you
    > call it and the way it crashes allows for some /assumptions/
    > (not statements, let's make this straight):
    >
    > join looks like some function, that (I presume it from the name)
    > concatenates a array of strings into a single string, putting
    > the string defined in the second argument between the parts.
    >
    > To indicate, that the last element has been reached, a NULL
    > pointer shall be provided as the terminating (in most cases this
    > is the last) array element.
    >
    > The function "test" supports this assumptions, as it's while loop
    > exhibits the very same termination logic. So to be correct, your
    > main-code should be:
    >
    > char ja[] = "This";
    > char jo[] = "That";
    > char je[] = "Nothing";
    >
    > char *text[4];
    >
    > text[0] = ja;
    > text[1] = jo;
    > text[2] = je;
    > text[3] = NULL; /* NULL == 0 BTW */
    >
    > join(erg, "<", text)
    >
    > NOTE: The interface of "join" and "test" are both inerhently
    > unsafe: They both assume, that the 'buffer' argument points to a
    > memory location with enough space allocated to hold the result.
    >
    > Even if the functions have a hard coded limit, so that you can't
    > overrun the buffer with 'overlong' strings in the 's2' argument,
    > it's still possible to give a too short buffer, thus causing a
    > overrun. Buffer overruns have been the most common cause for
    > security holes, as they allow to inject code into a program
    > (google for "buffer overrun exploit techniques").
    >
    > Wolfgang Draxinger
    > --
    > E-Mail address works, Jabber: , ICQ: 134682867


    Thanks! This code is in /pre/ stadium so i know it doesn't have any
    security checks but i think its better to use a pointer array as an
    argument instead of a double pointer.

    Now it works.
     
    tfelb, Jan 13, 2009
    #4
  5. tfelb wrote:

    > Thanks! This code is in /pre/ stadium so i know it doesn't have
    > any security checks


    Epic failure!

    Security is difficult to add later in existing systems. Always, I
    repeat, always design interfaces in a

    > but i think its better to use a pointer
    > array as an argument instead of a double pointer.


    You mean something like

    char *s2[]

    ? Technically it behaves exactly like char **s2, though at
    compilation stage the types subtely differ. However this is
    something of interest for compiler writers, the average coder
    should know, that the "pointer to pointer" notation is almost
    the same as the "array of pointers" notation and is identical,
    when it comes to machine language.

    > Now it works.


    It works in if well formulated data is input. A much safer
    interface would be:

    void join(
    unsigned int const dst_buffer_length,
    char * const dst_buffer,
    char const * const delimiter_string,
    unsigned int const src_string_count
    char const * const * const src_strings,
    );


    A even more safer style would completely abandon the use
    of "naked" arrays and instead encapsulate every array operation
    within bounds checking helper functions. Have a look at Felix
    von Leitner's libowfat library: It provides such an array
    abstraction - together with things like overflow safe integer
    multiplication and other neat stuff.
    <http://www.fefe.de/libowfat/>

    Just to give you an idea how it works with arrays, this is an
    excerpt of "array.h" of libowfat:

    #ifndef ARRAY_H
    #define ARRAY_H

    #include "uint64.h"

    typedef struct {
    char* p;
    int64 allocated; /* in bytes */
    uint64 initialized; /* in bytes */

    /* p and allocated nonzero: array is allocated */
    /* p and allocated zero: array is unallocated */
    /* p zero and allocated < 0: array is failed */
    } array;

    void* array_allocate(array* x,uint64 membersize,int64 pos);
    void* array_get(array* x,uint64 membersize,int64 pos);
    void* array_start(const array* const x);
    int64 array_length(const array* const x,uint64 membersize);
    int64 array_bytes(const array* const x);
    void array_truncate(array* x,uint64 membersize,int64 len);
    void array_trunc(array* x);
    void array_reset(array* x);
    void array_fail(array* x);
    /* ... */

    uint64.h is the header to overflow safe multiplication code and
    associated types (it wont resort to bignums, if a multiplication
    overflows, but such is detected and triggers error handling).
    Thow only "drawback" of libowfat is, that it's GPL licenced, so
    you can't use it in a closed source project. But you're still
    free to have a look at it's code and get inspiration from it.

    Wolfgang Draxinger
    --
    E-Mail address works, Jabber: , ICQ: 134682867
     
    Wolfgang Draxinger, Jan 13, 2009
    #5
  6. Wolfgang Draxinger <> writes:
    > tfelb wrote:

    [...]
    >> but i think its better to use a pointer
    >> array as an argument instead of a double pointer.


    (Note: "double pointer" here means pointer to pointer, not type
    double*.)

    > You mean something like
    >
    > char *s2[]
    >
    > ? Technically it behaves exactly like char **s2, though at
    > compilation stage the types subtely differ. However this is
    > something of interest for compiler writers, the average coder
    > should know, that the "pointer to pointer" notation is almost
    > the same as the "array of pointers" notation and is identical,
    > when it comes to machine language.

    [...]

    It's actually simpler than that. The parameter declaration

    char *s2[];

    is identical to

    char **s2;

    It's not "almost the same"; it is the same, not just in machine
    language but in C. Obviously the compiler has to handle the
    translation of the former to the latter.

    (Other than in a parameter declaration, of course, pointers and arrays
    are very different, though some features of the language seemingly
    conspire to hide that fact. Section 6 of the comp.lang.c FAQ explains
    this well.)

    --
    Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    Nokia
    "We must do something. This is something. Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"
     
    Keith Thompson, Jan 13, 2009
    #6
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Antonio

    Strange encoding behaviour

    Antonio, Dec 29, 2004, in forum: ASP .Net
    Replies:
    0
    Views:
    427
    Antonio
    Dec 29, 2004
  2. Jan
    Replies:
    2
    Views:
    1,438
    Mike Treseler
    Dec 16, 2004
  3. David Cantin

    Strange behaviour with perl and apache

    David Cantin, Nov 3, 2003, in forum: Perl
    Replies:
    1
    Views:
    456
    Jim Gibson
    Nov 3, 2003
  4. Dennis Johansson
    Replies:
    1
    Views:
    499
    Dennis Johansson
    Aug 21, 2003
  5. Andy Chambers
    Replies:
    1
    Views:
    388
    Daniel Dyer
    May 14, 2007
Loading...

Share This Page