$ hg id
3736bf94535c+ tip

-Wformat=2 -Wformat-security are useful for detecting possible security related bugs. Compiling with the two options produced a few hits in the source code.

/usr/bin/gcc -pthread -c -Wno-unused-result -Werror=declaration-after-statement -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -fno-common -Wstrict-overflow -Wformat=2 -Wformat-security -Wcast-align  -Wtrampolines  -fno-common -Wstrict-overflow -Wformat=2 -Wformat-security -Wcast-align  -Wtrampolines    -I. -IInclude -I./Include    -DPy_BUILD_CORE -o Objects/unicodeobject.o cpython/./Objects/unicodeobject.c
cpython/./Objects/unicodeobject.c: In function ‘unicode_fromformat_arg’:
cpython/./Objects/unicodeobject.c:2527:25: warning: format not a string literal, argument types not checked [-Wformat-nonliteral]
cpython/./Objects/unicodeobject.c:2531:25: warning: format not a string literal, argument types not checked [-Wformat-nonliteral]
cpython/./Objects/unicodeobject.c:2535:25: warning: format not a string literal, argument types not checked [-Wformat-nonliteral]
cpython/./Objects/unicodeobject.c:2538:25: warning: format not a string literal, argument types not checked [-Wformat-nonliteral]
cpython/./Objects/unicodeobject.c:2542:13: warning: format not a string literal, argument types not checked [-Wformat-nonliteral]
cpython/./Objects/unicodeobject.c:2549:25: warning: format not a string literal, argument types not checked [-Wformat-nonliteral]
cpython/./Objects/unicodeobject.c:2553:25: warning: format not a string literal, argument types not checked [-Wformat-nonliteral]
cpython/./Objects/unicodeobject.c:2557:25: warning: format not a string literal, argument types not checked [-Wformat-nonliteral]
cpython/./Objects/unicodeobject.c:2560:25: warning: format not a string literal, argument types not checked [-Wformat-nonliteral]

I think those are necessary for to `unicode_fromformat_arg`.

/usr/bin/gcc -pthread -c -Wno-unused-result -Werror=declaration-after-statement -DNDEBUG -g -fwrapv -O3 -Wall -Wstrict-prototypes -fno-common -Wstrict-overflow -Wformat=2 -Wformat-security -Wcast-align  -Wtrampolines  -fno-common -Wstrict-overflow -Wformat=2 -Wformat-security -Wcast-align  -Wtrampolines    -I. -IInclude -I./Include    -DPy_BUILD_CORE -o Modules/main.o cpython/./Modules/main.c
cpython/./Modules/main.c: In function ‘usage’:
cpython/./Modules/main.c:111:5: warning: format not a string literal, argument types not checked [-Wformat-nonliteral]
cpython/./Modules/main.c:118:9: warning: format not a string literal, argument types not checked [-Wformat-nonliteral]
cpython/./Modules/main.c:119:9: warning: format not a string literal, argument types not checked [-Wformat-nonliteral]

I think the occurrences in main.c could benefit from "%s" to ensure the program does not accidentally leak.

If interested, I think the warnings can be selectively turned off:

#if defined (__GNUC__) && ((__GNUC__ == 4 && __GNUC_MINOR__ >= 6) || (__GNUC__ >= 5))
# pragma GCC diagnostic push
# pragma GCC diagnostic ignored "-Wformat-security"
#endif

unicode_fromformat_arg(...)
{
   ...
}

#if defined (__GNUC__) && ((__GNUC__ == 4 && __GNUC_MINOR__ >= 6) || (__GNUC__ >= 5))
# pragma GCC diagnostic pop
#endif

Microsoft has a similar mechanism.

It should allow the project to compile with -Wformat-security full time while maintinaing a quiet compile (silent is good).

> #if defined (__GNUC__) && ((__GNUC__ == 4 && __GNUC_MINOR__ >= 6) || (__GNUC__ >= 5))
> # pragma GCC diagnostic push
> # pragma GCC diagnostic ignored "-Wformat-security"
> #endif

My bad... -Wformat-nonliteral

New changeset d6d2549340cb by Victor Stinner in branch 'default':
Issue #20948: Inline makefmt() in unicode_fromformat_arg()
https://hg.python.org/cpython/rev/d6d2549340cb

The format parameter passed to sprintf() is created by makefmt() function. In Python 3.5, makefmt() has a few parameters. The code is simple and looks safe.

The makefmt() function was much more complex in Python 3.3, it had more parameters: zeropad, width and precision. I refactored PyUnicode_FromFormatV() to optimize it. During the optimization, makefmt() was simplified, and in fact it is now possible to inline it and remove it. I just removed it in Python 3.5.

Should we change something in Python 2.7 and 3.4? Ignore the warning? Or can I just close the issue?

Thanks for the report Jeffrey.

The Modules/main.c cases are not errors. They are just long strings defined as static constants, rather than literals passed in directly.

I think we can close this now. Unless people think this warning is worth using, in which case we should find a way to work around the false positives.

On Tue, Jul 26, 2016 at 4:31 AM, Martin Panter <report@bugs.python.org> wrote:
>
> Martin Panter added the comment:
>
> The Modules/main.c cases are not errors. They are just long strings defined as static constants, rather than literals passed in directly.
>
> I think we can close this now. Unless people think this warning is worth using, in which case we should find a way to work around the false positives.
>

Would it be possible to add some instrumentation to silence the
finding? There's no sense in having multiple developers and qa
research the issue. I'm guessing a percentage of developers and qa
will file bug reports, so it will burn some of the python team's
cycles, too.

Maybe something like:

#if (GCC_VERSION >= 40600) || (LLVM_CLANG_VERSION >= 10700) ||
(APPLE_CLANG_VERSION >= 20000)
#  define GCC_DIAGNOSTIC_AVAILABLE 1
#endif

#if GCC_DIAGNOSTIC_AVAILABLE
#  pragma GCC diagnostic ignored "-Wformat-security"
#endif

If its safe to ignore the warning, then the technique above should be
safe for a C/CC/CXX/CPP files. It will not cross-pollinate because its
a source file, and not a header file.

Jeff

Seems warnings are gone after adding the const qualifier to static constant arrays (issue25923).

GCC’s -Wformat options are documented at <https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wformat-321>. We already enable -Wall by default, which implicitly enables -Wformat, but not -Wformat=2. Apparently, -Wformat=2 enables -Wformat-security, so you shouldn’t have to manually enable it.

The -Wformat-security option enables warnings for stuff like printf(variable), without any extra arguments. Such a call could easily be changed, even if it is not a genuine problem. So I wouldn’t have a problem enabling this warning by default. (See also Issue 23545 proposing a few other warnings.)

However I am not convinced it is worth working around the -Wformat-nonliteral warnings, given that all the warnings given above were false positives, and the general workaround would need six or seven preprocessor lines.