[Cscope-devel] [ cscope-Bugs-1064875 ] buffer overflow vulnerabilities

[SourceForge.net <no...@so...>]

Bugs item #1064875, was opened at 2004-11-11 19:14
Message generated for change (Comment added) made by nhorman
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=104664&aid=1064875&group_id=4664

Category: None
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Jason Duell (jcduell)
Assigned to: Nobody/Anonymous (nobody)
Summary: buffer overflow vulnerabilities

Initial Comment:
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.





----------------------------------------------------------------------

Comment By: Neil Horman (nhorman)
Date: 2004-12-09 13:12

Message:
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.

----------------------------------------------------------------------

Comment By: Hans-Bernhard Broeker (broeker)
Date: 2004-12-09 11:29

Message:
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.

----------------------------------------------------------------------

Comment By: Neil Horman (nhorman)
Date: 2004-12-09 11:11

Message:
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?

----------------------------------------------------------------------

Comment By: Hans-Bernhard Broeker (broeker)
Date: 2004-12-09 10:36

Message:
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.

----------------------------------------------------------------------

Comment By: Neil Horman (nhorman)
Date: 2004-12-08 15:53

Message:
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?

----------------------------------------------------------------------

Comment By: Hans-Bernhard Broeker (broeker)
Date: 2004-12-08 15:14

Message:
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.

----------------------------------------------------------------------

Comment By: Neil Horman (nhorman)
Date: 2004-12-07 13:38

Message:
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? :)

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=104664&aid=1064875&group_id=4664