[libcxx-commits] [PATCH] D112927: [libc++] Enable -Wformat-nonliteral when building libc++

[Louis Dionne via Phabricator via libcxx-commits]

ldionne created this revision.
Herald added a subscriber: mgorny.
ldionne requested review of this revision.
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.
Herald added a reviewer: libc++.

Using user-provided data as a format string is a well known source of
security vulnerabilities. For this reason, it is a good idea to compile
our code with -Wformat-nonliteral, which basically warns if a non-constant
string is used as a format specifier. This is the compiler’s best signal
that a format string call may be insecure.

I audited the code after adding the warning and made sure that the few
places where we used a non-literal string as a format string were not
potential security issues. I disabled the warning locally for those
instances. The idea is that after we add the warning to the build, any
new use of a non-literal string in a format string will trigger a
diagnostic, and we can either get rid of it or disable the warning
locally, which is a way of acknowledging that it has been audited.

I also looked into enabling it in the test suite, which would perhaps
allow finding additional instances of it in our headers, however that
is not possible at the moment because Clang doesn't support putting
__attribute__((__format__(...))) on variadic templates, which would
be needed.

rdar://84571685


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112927

Files:
  libcxx/CMakeLists.txt
  libcxx/include/locale


Index: libcxx/include/locale
===================================================================
--- libcxx/include/locale
+++ libcxx/include/locale
@@ -1486,7 +1486,10 @@
         + ((numeric_limits<_Unsigned>::digits % 3) != 0) // round up
         + 2; // base prefix + terminating null character
     char __nar[__nbuf];
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wformat-nonliteral"
     int __nc = __libcpp_snprintf_l(__nar, sizeof(__nar), _LIBCPP_GET_C_LOCALE, __fmt, __v);
+#pragma clang diagnostic pop
     char* __ne = __nar + __nc;
     char* __np = this->__identify_padding(__nar, __ne, __iob);
     // Stage 2 - Widen __nar while adding thousands separators
@@ -1546,6 +1549,8 @@
     char __nar[__nbuf];
     char* __nb = __nar;
     int __nc;
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wformat-nonliteral"
     if (__specify_precision)
         __nc = __libcpp_snprintf_l(__nb, __nbuf, _LIBCPP_GET_C_LOCALE, __fmt,
                                    (int)__iob.precision(), __v);
@@ -1562,6 +1567,7 @@
             __throw_bad_alloc();
         __nbh.reset(__nb);
     }
+#pragma clang diagnostic pop
     char* __ne = __nb + __nc;
     char* __np = this->__identify_padding(__nb, __ne, __iob);
     // Stage 2 - Widen __nar while adding thousands separators
@@ -1609,7 +1615,10 @@
     char __fmt[6] = "%p";
     const unsigned __nbuf = 20;
     char __nar[__nbuf];
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wformat-nonliteral"
     int __nc = __libcpp_snprintf_l(__nar, sizeof(__nar), _LIBCPP_GET_C_LOCALE, __fmt, __v);
+#pragma clang diagnostic pop
     char* __ne = __nar + __nc;
     char* __np = this->__identify_padding(__nar, __ne, __iob);
     // Stage 2 - Widen __nar
Index: libcxx/CMakeLists.txt
===================================================================
--- libcxx/CMakeLists.txt
+++ libcxx/CMakeLists.txt
@@ -614,7 +614,8 @@
   endif()
   target_add_compile_flags_if_supported(${target} PRIVATE -Wextra -W -Wwrite-strings
                                                           -Wno-unused-parameter -Wno-long-long
-                                                          -Werror=return-type -Wextra-semi -Wundef)
+                                                          -Werror=return-type -Wextra-semi -Wundef
+                                                          -Wformat-nonliteral)
   if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang")
     target_add_compile_flags_if_supported(${target} PRIVATE
       -Wno-user-defined-literals


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D112927.383798.patch
Type: text/x-patch
Size: 2556 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20211101/a3a47fc7/attachment.bin>