From: SourceForge.net <no...@so...> - 2004-12-09 18:43:23
|
Bugs item #1064875, was opened at 2004-11-12 01:14 Message generated for change (Comment added) made by broeker 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: Hans-Bernhard Broeker (broeker) Date: 2004-12-09 19:43 Message: 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. ---------------------------------------------------------------------- Comment By: Neil Horman (nhorman) Date: 2004-12-09 19: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 17: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 17: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 16: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 21: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 21: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 19: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 |