views:

693

answers:

2

We have a Coverity bug for this line of code:

snprintf( tempStr, size, testStrings[testID], A2DtoV(testResults[testID].value),
A2DtoV(testResults[testID].min),A2DtoV(testResults[testID].max));

The error says:

non_const_printf_format_string: "format string is not a string literal, 
potential security vulnerability if user controlled"

I changed testStrings to a const, but that didn't do anything:

static const char *testStrings[] = {"1", ... etc};

Any ideas as to what this error is really saying?

+2  A: 

Idea is that value of testStrings[testID] can be changed somehow to include extra format specifiers.

Because snprintf() has no possibility to check whether number of parameters match the number of format specifiers it will just take next address from stack to use as value for next format specifier and weird things can happen then.

It is known as format string attack.

qrdl
+6  A: 

Your code is fine.

The issue is that if you pass a string that is user controlled as a printf format string, security bugs can arise.

For instance, printf(userName);

Where userName is supplied by the user, a user can pass "%s", and get your function to start accessing data at a random address on the stack, which could result in a crash. printf will try to pop additional parameters off the stack, resulting in a stack corruption. Denial of service attack like this is probably the best case, information can be disclosed by getting printf to dump out values on the stack and there are even ways to get printf style functions to modify the return address on the stack.

Since your strings are not user controlled, it is safe to ignore this message. The typical fix is to replace the printf example I gave with printf("%s", userName);, which would not appear to help in your case because the const strings appear to contain format strings.

Wikipedia has more on format string vulnerabilities here: http://en.wikipedia.org/wiki/Format_string_vulnerabilities

Michael