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 > [EMAIL PROTECTED] > > > > _______________________________________________ > cfe-dev mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev _______________________________________________ cfe-dev mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
