[cfe-dev] vprintf(3) and "format string is not a string literal"

[Ted Kremenek]

Hi Shantonu,

We probably should make these kinds of warnings optional.  In  case  
you are not aware, format-string attacks are an extremely serious and  
often exploited attack vector for a hacker to compromise a system, and  
have been the source of much grief.  Here are a couple references on  
the issue:

http://en.wikipedia.org/wiki/Format_string_attack
http://citeseer.ist.psu.edu/lhee02buffer.html (see the PDF link)

Essentially the idea is that if the format string is a value whose  
contents can be dynamically controlled at runtime (and not statically  
specified via a string literal), if a hacker can control the contents  
of the format string they can induce a buffer overrun, inject  
shellcode into a process, and hijack control of the process by jumping  
to that shellcode.  The can happen because certain format specifiers  
actually allow you to write data back to a buffer (which is extremely  
dangerous in any context).  Consequently, casual use of format strings  
specified via a function variable are generally discouraged by  
security experts, and this is the motivation behind this warning.  The  
warning, however, doesn't always indicate an actually security hole or  
exploit; it just warns of a "deprecated" API.

Clearly the logmessage function in your example code snippet is a  
possible exception because it serves as a wrapper to vprintf (so this  
warning could be considered a false positive).  If the format string  
checker was more powerful (i.e., it performed an inter-procedural  
program analysis), it should similarly flag casual uses of the  
logmessage function where the format string is not specified with a  
string constant.

Your question specifically regarding vprintf is an interesting one.   
Clearly vprintf was designed with the idea of passing in arguments  
from some other source, be it via parameters from a caller of the  
current function or pulled from a data structure.  Depending on how  
careful you are, this can be okay (such as with the logmessage  
function), but in general it's just a bad idea.  The va_list argument  
to vprintf (and friends) allows a tremendous amount of flexibility in  
how these functions are used (specifically be a dynamically specified  
argument list almost always implies a dynamically specified format  
string); the horribly consequence is that this code is very difficult  
to check statically for correctness, and can be the source of awful  
security holes and other bugs when these functions aren't used  
properly.  Unfortunately, people often underestimate how easy it is to  
screw up how these functions are used, either when they are called  
directly or called indirectly via wrappers.

Ultimately, we should probably make warnings like these an option.   
People can then decide their own policy on when such warnings are  
emitted.

Ted

On Dec 16, 2007, at 8:03 PM, Shantonu Sen wrote:

> I don't understand the following warning:
>
> $ cat a.c
> #include <stdarg.h>
> #include <stdio.h>
>
> int logmessage(int loglevel, char const *fmt, ...) {
>     int ret = 0;
>     va_list ap;
>
>     if (loglevel > 1) {
>         va_start(ap, fmt);
>         ret = vprintf(fmt, ap);
>         va_end(ap);
>     }
>     return ret;
> }
> $ clang -std=c99 a.c
> a.c:10:23: warning: format string is not a string literal (potentially
> insecure)
>         ret = vprintf(fmt, ap);
>               ~~~~~~~ ^
> 1 diagnostic generated.
>
> This seems counter-intuitive to the point of the vprintf(3) API, which
> is to pass the format string and arguments from its caller
> (logmessage()) in this case. When would vprintf(3) ever realistically
> be called with a string literal? There seems to be test cases and
> explicit code for this, so I'm guessing this is intentional, but I
> don't quite understand why...
>
> Shantonu Sen
> ssen at apple.com
>
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev