Menu

#154 buffer overflow vulnerabilities

closed-out-of-date
None
5
2006-10-15
2004-11-12
Jason Duell
No

While we're on the topic of security issues:

Cscope contains an alarming number of buffer overflow
vulnerabilities. By a rough count, there are at least
48 places where we blindly sprintf() a file name into a
fixed-length buffer of size PATHLEN without checking to
see if the file's name is <= PATHLEN. We do similar
things with environment variable values. Running an
automated tool for detecting these sort of things
(RATS:
http://www.securesoftware.com/resources/tools.html\)
reports 79 warnings about potentially insecure uses of
fixed-length buffers.

Most of these buffer overflow possibilities concern
things that are under user control (like environment
variables and most filenames), so they're not so
serious, but there's at least one place where a
malicious entity could cause more trouble: when we
parse source code, we don't check the length of
#include <filename> before we copy it into a PATHLEN
buffer. If someone managed to hack into a site that
distributes widely-browsed source code (kernel.org,
say), they could conceivably replace the source code
tarball with one that's been modified to contain a
carefully constructed #include that implements a buffer
overflow attack on cscope. This is not terribly
likely, and would only be feasible against a particular
binary of cscope (i.e. most likely against a particular
RPM version, for widest impact), but we should fix it.

Discussion

  • Neil Horman

    Neil Horman - 2004-12-07

    Logged In: YES
    user_id=827328

    It sounds like most of this could be fixed by a recursive:
    sed -e's/\(sprintf\)\(.*\,)\(.*\)/snprintf\2PATHLEN,\3/'

    Does that sound about right? :)

     
  • Hans-Bernhard Broeker

    Logged In: YES
    user_id=27517

    It's not quite that simple --- snprintf() isn't reliably
    available, so we can't use it without a fall-back plan.

     
  • Neil Horman

    Neil Horman - 2004-12-08

    Logged In: YES
    user_id=827328

    well, configure already determines if snprintf is available
    and sets HAVE_SNPRINTF accordingly, so what if we define our
    own snprintf in the event that HAVE_SNPRINTF isn't defined
    to check lengths on our own?

     
  • Hans-Bernhard Broeker

    Logged In: YES
    user_id=27517

    Defining your own snprintf is surprisingly hard --- that's
    the main reason it's an ISO C99 requirement now. It's
    essentially impossible to implement a complete snprintf() in
    user code. About the only portable global modification that
    works is

    #if !HAVE_SNPRINTF
    # define snprintf plastic_snprintf
    #endif

    and then implement a cheap plastic version of snprintf that
    just calls vsprintf in turn and simply drops the bufsiz
    argument on the way. If you lack vsprintf(), too, you're toast.

     
  • Neil Horman

    Neil Horman - 2004-12-09

    Logged In: YES
    user_id=827328

    How many platforms do you think we have at this point that
    don't have snprintf? Is it worth perhaps just adding a
    message to the configure stage or a #warning during the
    build to indicate that the lack of snprintf means the
    resultant binary can be open to various buffer overflow attacks?

     
  • Hans-Bernhard Broeker

    Logged In: YES
    user_id=27517

    We have at least one such platform: DJGPP V2.03, the MSDOS
    port of GCC & friends (binaries also run on Windows, and can
    be preferrable to Cygwin for various reasons.)

    A warning at configure or compile time wouldn't suffice: if
    snprintf() isn't there, you need either variadic macro
    support (i.e. a C99-alike compiler) or vsprintf to work
    around that, unless you're willing to re-work every single
    call of sprintf() --- in which case it loses a good part of
    its appeal. Unless you want to do something truly ugly, like

    #if HAVE_SNPRINTF
    # define nsize(size) , size
    #else
    # define snprintf sprintf
    # define nsize(size) , /* nothing */
    #endif

    and obfuscate all calls to snprintf like this:

    snprintf(buf nsize(bufsize), "%format", ...)

    I don't want to do that. Not if we can avoid it at almost
    any cost.

     
  • Neil Horman

    Neil Horman - 2004-12-09

    Logged In: YES
    user_id=827328

    what about a completely different approach for systems
    without snprintf? Perhaps a canary type defense? Is it
    possible under windows to make memory pages non-writeable
    (a-la mprotect)? If so, we could wrap snprintf in a wrapper
    function that allocates an extra stack page and a static
    buffer which it uses in the wrapped call to sprintf. We
    mprotect teh extra page to be read-only, so that if the
    underlying call overwrites the static buffer, we catch a
    segfault. It still crashes the system, but we avoid the
    potential for expoits. Its kinda slow, but it solves the
    problem for systems that don't have snprintf.

     
  • Hans-Bernhard Broeker

    Logged In: YES
    user_id=27517

    No. First of all, the current main entry in our list of
    "systems without snprintf" is DOS, not Windows --- but
    anway, whatever we do should *portable*. Second, no,
    DOS/DPMI doesn't generally offer read-only memory. Third,
    rolling such a system on our own would be almost as serious
    overkill as writing our own snprintf.

    Anyway, I'm reasonably sure we don't need snprintf() for all
    the problematic calls we have right now. Most of them can
    be fixed by specifying explicit field widths, I think.

     
  • Jason Duell

    Jason Duell - 2004-12-11

    Logged In: YES
    user_id=125727

    There do appear to be a number of portable snprintf
    implementations out there:

    http://www.ijs.si/software/snprintf/

    I'm guessing that the apache one (see links a the bottom) is
    probably both well tested, and license-compatible with cscope.

     
  • Neil Horman

    Neil Horman - 2004-12-11

    Logged In: YES
    user_id=827328

    I'll test it out this weekend.

     
  • Hans-Bernhard Broeker

    Logged In: YES
    user_id=27517

    The majority of these should be fixed now. Closing this
    report. If anything is still critical, open a new report.

     
  • Hans-Bernhard Broeker

    • assigned_to: nobody --> broeker
    • status: open --> closed-out-of-date
     
  • Nobody/Anonymous

    Mu5riK <a href="http://teslcjckkydb.com/">teslcjckkydb</a>, [url=http://zzlauxetmaer.com/]zzlauxetmaer[/url], [link=http://fbxnqvqnidwo.com/]fbxnqvqnidwo[/link], http://zsnflexfvkyp.com/

     

Log in to post a comment.

Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.