What is the potential problem?

Discussion in 'C Programming' started by happy, Jan 2, 2010.

  1. happy

    happy Guest

    I read an unanswered interview question somewhere :

    #include <stdlib.h>
    #include <stdio.h>
    void Error(char* s)
    {
    printf(s);
    return;
    }

    int main()
    {
    int *p;
    p = (int*)malloc(sizeof(int));
    if(!p)
    {
    Error("memory error");
    Error("Quitting....\n");
    exit(1);
    }
    else
    {
    /*some stuff to use p*/
    }
    free(p);
    return 0;
    }

    Find potential problem with the error function.. a security concern..

    Please tell me what can be the potential problem. Is this the printf
    (p) because p can contain conversion specifiers?
    happy, Jan 2, 2010
    #1
    1. Advertising

  2. happy

    Stefan Ram Guest

    happy <> writes:
    >Please tell me what can be the potential problem. Is this the printf
    >(p) because p can contain conversion specifiers?


    Yes, but if the function would be »static«, it would be more secure,
    because then it would not be possible to call it from other linking
    units, and in /this/ compilation unit, it is only called in:

    >Error("memory error");
    >Error("Quitting....\n");


    where there are no conversion specifiers visible.
    Stefan Ram, Jan 2, 2010
    #2
    1. Advertising

  3. happy

    bartc Guest

    "Stefan Ram" <-berlin.de> wrote in message
    news:-berlin.de...
    > happy <> writes:
    >>Please tell me what can be the potential problem. Is this the printf
    >>(p) because p can contain conversion specifiers?

    >
    > Yes, but if the function would be »static«, it would be more secure,
    > because then it would not be possible to call it from other linking
    > units, and in /this/ compilation unit, it is only called in:
    >
    >>Error("memory error");
    >>Error("Quitting....\n");

    >
    > where there are no conversion specifiers visible.


    An external routine may not know that "%" characters are special in the
    message strings passed to Error().

    But that they would just make it go wrong, I can't see a security issue. And
    if the caller of Error() was malicious, he could just call his own printf()
    function with whatever he likes.

    --
    Bartc
    bartc, Jan 2, 2010
    #3
  4. happy

    Eric Sosman Guest

    On 1/2/2010 1:06 PM, happy wrote:
    > I read an unanswered interview question somewhere :
    >
    > #include<stdlib.h>
    > #include<stdio.h>
    > void Error(char* s)
    > {
    > printf(s);
    > return;
    > }
    >
    > int main()
    > {
    > int *p;
    > p = (int*)malloc(sizeof(int));
    > if(!p)
    > {
    > Error("memory error");
    > Error("Quitting....\n");
    > exit(1);
    > }
    > else
    > {
    > /*some stuff to use p*/
    > }
    > free(p);
    > return 0;
    > }
    >
    > Find potential problem with the error function.. a security concern..
    >
    > Please tell me what can be the potential problem. Is this the printf
    > (p) because p can contain conversion specifiers?


    That would be my guess at what the question's author was
    trying to get at. The question is poorly framed, though,
    since we don't have enough context. There is no "security
    concern," for example, if all the Error strings are provided
    by a trusted source, someone who knows not to put percent
    signs in them.

    There are other potential problems, depending on what you
    consider a "problem." For example, the output might not appear
    at all if the caller of Error forgets to end with a '\n' (in
    the example, the output would be "memory errorQuitting....\n"
    because Error does not supply its own line break). Also, Error
    sends its messages to stdout, whereas they probably ought to go
    to stderr.

    There's also a useless `return' statement, and the caller
    has a useless cast and a non-portable exit status. All in all,
    I'd say the question is poorly composed.

    --
    Eric Sosman
    lid
    Eric Sosman, Jan 2, 2010
    #4
  5. happy

    Stefan Ram Guest

    "bartc" <> writes:
    >An external routine may not know that "%" characters are special in the
    >message strings passed to Error().


    This could also be attributted to the lack of proper
    documentation of this function. Thus, documentation also has
    security benefits/implications. So, one answer to the
    original question might be: The lack of documentation makes
    it impossible to say whether the program has the intended
    behavior, because we do not know what it is intended to do.

    And not being able to say what a program is supposed to do
    might be deemed a security problem, because

    - failures in the implementation might go unnoticed and

    - the program might be used/called under circumstances
    or for reasons for which it has not been designed.

    >But that they would just make it go wrong, I can't see a security issue.


    If the string was read in, it could have been created
    in order to support an attack.
    Stefan Ram, Jan 2, 2010
    #5
  6. "bartc" <> writes:

    > But that they would just make it go wrong, I can't see a security
    > issue. And if the caller of Error() was malicious, he could just call
    > his own printf() function with whatever he likes.


    The security issue is if the caller is not malicious but is
    passing a string from a malicious source, e.g. if an authorized
    user is running the program to view some file on a floppy disk
    that he found lying on the street next to the office.

    Most conversion specifications in this printf call would just
    cause printf to read a va_arg that was not provided by the
    caller, and then either display it (which as such may already
    write some secret to a place that unauthorized people can see) or
    dereference it as a pointer and possibly crash (for "%s"), but
    "%n" is unusual in that it actually writes to memory. The
    attacker can control the value written (i.e. the number of
    characters output so far), and by suitably corrupting the stack
    or other data structures, he may be able to lure the program
    into executing some code of his choosing.
    Kalle Olavi Niemitalo, Jan 2, 2010
    #6
  7. happy

    Paul N Guest

    On 2 Jan, 18:43, Eric Sosman <> wrote:
    > On 1/2/2010 1:06 PM, happy wrote:
    >
    >
    >
    >
    >
    > > I read an unanswered interview question somewhere :

    >
    > > #include<stdlib.h>
    > > #include<stdio.h>
    > > void Error(char* s)
    > > {
    > > printf(s);
    > > return;
    > > }

    >
    > > int main()
    > > {
    > > int *p;
    > > p = (int*)malloc(sizeof(int));
    > > if(!p)
    > > {
    > > Error("memory error");
    > > Error("Quitting....\n");
    > > exit(1);
    > > }
    > > else
    > > {
    > > /*some stuff to use p*/
    > > }
    > > free(p);
    > > return 0;
    > > }

    >
    > > Find potential problem with the error function.. a security concern..

    >
    > > Please tell me what can be the potential problem. Is this the printf
    > > (p) because p can contain conversion specifiers?

    >
    >      That would be my guess at what the question's author was
    > trying to get at.  The question is poorly framed, though,
    > since we don't have enough context.  There is no "security
    > concern," for example, if all the Error strings are provided
    > by a trusted source, someone who knows not to put percent
    > signs in them.
    >
    >      There are other potential problems, depending on what you
    > consider a "problem."  For example, the output might not appear
    > at all if the caller of Error forgets to end with a '\n' (in
    > the example, the output would be "memory errorQuitting....\n"
    > because Error does not supply its own line break).  Also, Error
    > sends its messages to stdout, whereas they probably ought to go
    > to stderr.
    >
    >      There's also a useless `return' statement, and the caller
    > has a useless cast and a non-portable exit status.  All in all,
    > I'd say the question is poorly composed.


    In addition to what the others have said - the function "Error"
    doesn't actually stop the program, which one might expect it to do.
    Paul N, Jan 3, 2010
    #7
    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. Robert May

    Potential ASP.NET bug

    Robert May, Oct 16, 2003, in forum: ASP .Net
    Replies:
    1
    Views:
    410
    Robert May
    Oct 16, 2003
  2. Lloyd Sheen
    Replies:
    0
    Views:
    320
    Lloyd Sheen
    Jan 24, 2004
  3. alexonjava99
    Replies:
    0
    Views:
    1,736
    alexonjava99
    Jul 26, 2003
  4. Replies:
    8
    Views:
    635
    Jim Langston
    Aug 16, 2006
  5. bsder

    potential problem with fork()

    bsder, Nov 25, 2005, in forum: Perl Misc
    Replies:
    4
    Views:
    85
    John W. Krahn
    Nov 26, 2005
Loading...

Share This Page