build: Add hardening options (#4538)

Get rid of the FORTIFY options, which were too specific to one compiler
feature. Add OIIO_HARDENING that takes a level meaning:

- 0 : do nothing, not recommended.
- 1 : enable features that have no (or nearly no) performance impact,
recommended default for optimized, shipping code.
- 2 : enable features that trade off performance for security,
recommended for debugging or deploying in security-sensitive
environments.
- 3 : enable features that have a significant performance impact, only
recommended for debugging.

Default to 1 for optimized builds, 3 for debug builds (so will be
thoroughly tested by our sanitizer and other CI tests). Users that have
more stringent security requirements may choose to build with 2 even for
shipping code (they should benchmark to see if that is acceptable to
them).

These levels turn on a variety of compiler options that are recommended
for additional safety. We will add more as they are developed.

There are also some warning suppressions that needed to be added to code
in a few areas where it was unavoidable to use some constructs that
trigger the elevated safety checks.

Signed-off-by: Larry Gritz <[email protected]>

src/cmake/compiler.cmake

@@ -86,6 +86,9 @@ elseif (CMAKE_CXX_COMPILER_ID MATCHES "Intel")
     set (CMAKE_COMPILER_IS_INTEL 1)
     message (VERBOSE "Using Intel as the compiler")
 endif ()
+if (CMAKE_COMPILER_IS_GNUCC OR CMAKE_COMPILER_IS_CLANG)
+    set (COMPILER_IS_GCC_OR_ANY_CLANG TRUE)
+endif ()
 
 
 ###########################################################################
@@ -102,12 +105,12 @@ else ()
 endif()
 option (EXTRA_WARNINGS "Enable lots of extra pedantic warnings" OFF)
 if (NOT MSVC)
-    add_compile_options ("-Wall")
+    add_compile_options (-Wall -Wformat -Wformat=2)
     if (EXTRA_WARNINGS)
-        add_compile_options ("-Wextra")
+        add_compile_options (-Wextra)
     endif ()
     if (STOP_ON_WARNING)
-        add_compile_options ("-Werror")
+        add_compile_options (-Werror)
     endif ()
 endif ()
 
@@ -462,25 +465,60 @@ endif ()
 
 
 ###########################################################################
-# Fortification and hardening options
-#
-# In modern gcc and clang, FORTIFY_SOURCE provides buffer overflow checks
-# (with some compiler-assisted deduction of buffer lengths) for the following
-# functions: memcpy, mempcpy, memmove, memset, strcpy, stpcpy, strncpy,
-# strcat, strncat, sprintf, vsprintf, snprintf, vsnprintf, gets.
-#
-# We try to avoid these unsafe functions anyway, but it's good to have the
-# extra protection, at least as an extra set of checks during CI. Some users
-# may also wish to enable it at some level if they are deploying it in a
-# security-sensitive environment. FORTIFY_SOURCE=3 may have minor performance
-# impact, though FORTIFY_SOURCE=2 should not have a measurable effect on
-# performance versus not doing any fortification. All fortification levels are
-# not available in all compilers.
-
-set (FORTIFY_SOURCE "0" CACHE STRING "Turn on Fortification level (0, 1, 2, 3)")
-if (FORTIFY_SOURCE AND (CMAKE_COMPILER_IS_GNUCC OR CMAKE_COMPILER_IS_CLANG))
-    message (STATUS "Compiling with _FORTIFY_SOURCE=${FORTIFY_SOURCE}")
-    add_compile_options (-D_FORTIFY_SOURCE=${FORTIFY_SOURCE})
+# Safety/security hardening options
+#
+# Explanation of levels:
+#  0 : do nothing, not recommended.
+#  1 : enable features that have no (or nearly no) performance impact,
+#      recommended default for optimized, shipping code.
+#  2 : enable features that trade off performance for security, recommended
+#      for debugging or deploying in security-sensitive environments.
+#  3 : enable features that have a significant performance impact, only
+#      recommended for debugging.
+#
+# Some documentation:
+# https://best.openssf.org/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C++.html
+# https://www.gnu.org/software/libc/manual/html_node/Source-Fortification.html
+# https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html
+# https://libcxx.llvm.org/Hardening.html
+#
+if (${CMAKE_BUILD_TYPE} STREQUAL "Debug")
+    set (${PROJ_NAME}_HARDENING_DEFAULT 3)
+else ()
+    set (${PROJ_NAME}_HARDENING_DEFAULT 1)
+endif ()
+set_cache (${PROJ_NAME}_HARDENING ${${PROJ_NAME}_HARDENING_DEFAULT}
+           "Turn on security hardening features 0, 1, 2, 3")
+# Implementation:
+if (HARDENING GREATER_EQUAL 1)
+    # Features that should not detectably affect performance
+    if (COMPILER_IS_GCC_OR_ANY_CLANG)
+        # Enable PIE and pie to build as position-independent executables,
+        # needed for address space randomiztion used by some kernels.
+        add_compile_options (-fPIE -pie)
+        add_link_options (-fPIE -pie)
+        # Protect against stack overwrites. Is allegedly not a performance
+        # tradeoff.
+        add_compile_options (-fstack-protector-strong)
+        add_link_options (-fstack-protector-strong)
+    endif ()
+    # Defining _FORTIFY_SOURCE provides buffer overflow checks in modern gcc &
+    # clang with some compiler-assisted deduction of buffer lengths) for the
+    # many C functions such as memcpy, strcpy, sprintf, etc. See:
+    add_compile_definitions (_FORTIFY_SOURCE=${${PROJ_NAME}_HARDENING})
+    # Setting _LIBCPP_HARDENING_MODE enables various hardening features in
+    # clang/llvm's libc++ 18.0 and later.
+    add_compiler_definitions (_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_FAST)
+endif ()
+if (HARDENING GREATER_EQUAL 2)
+    # Features that might impact performance measurably
+    add_compile_definitions (_GLIBCXX_ASSERTIONS)
+    add_compiler_definitions (_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_EXTENSIVE)
+endif ()
+if (HARDENING GREATER_EQUAL 3)
+    # Debugging features that might impact performance significantly
+    add_compile_definitions (_GLIBCXX_DEBUG)
+    add_compiler_definitions (_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG)
 endif ()
 
 

src/cmake/set_utils.cmake

@@ -98,9 +98,9 @@ macro (super_set name value)
         set (${name} ${_sce_val} ${_sce_extra_args})
     endif ()
     if (_sce_VERBOSE)
-        message (STATUS "${name} = ${${name}}")
+        message (STATUS "${name} = ${${name}} (${_sce_DOC})")
     else ()
-        message (VERBOSE "${name} = ${${name}}")
+        message (VERBOSE "${name} = ${${name}} (${_sce_DOC})")
     endif ()
     if (_sce_ADVANCED)
         mark_as_advanced (${name})

src/libutil/SHA1.cpp

@@ -269,10 +269,13 @@ bool CSHA1::ReportHash(TCHAR* tszReport, REPORT_TYPE rtReportType) const
 		_sntprintf(tszTemp, 15, _T("%02X"), m_digest[0]);
 		Strutil::safe_strcpy(tszReport, tszTemp, HASH_TEMP_BUFFER_SIZE - 1);
 
-		const TCHAR* lpFmt = ((rtReportType == REPORT_HEX) ? _T(" %02X") : _T("%02X"));
+		// const TCHAR* lpFmt = ((rtReportType == REPORT_HEX) ? _T(" %02X") : _T("%02X"));
 		for(size_t i = 1; i < 20; ++i)
 		{
-			_sntprintf(tszTemp, 15, lpFmt, m_digest[i]);
+			if (rtReportType == REPORT_HEX)
+				_sntprintf(tszTemp, 15, _T(" %02X"), m_digest[i]);
+			else
+				_sntprintf(tszTemp, 15, _T("%02X"), m_digest[i]);
 			Strutil::safe_strcat(tszReport, tszTemp, HASH_TEMP_BUFFER_SIZE - 1);
 		}
 	}

src/tiff.imageio/tiffinput.cpp

@@ -478,7 +478,10 @@ class TIFFInput final : public ImageInput {
     {
         TIFFInput* self = (TIFFInput*)user_data;
         spin_lock lock(self->m_last_error_mutex);
+        OIIO_PRAGMA_WARNING_PUSH
+        OIIO_GCC_PRAGMA(GCC diagnostic ignored "-Wformat-nonliteral")
         self->m_last_error = Strutil::vsprintf(fmt, ap);
+        OIIO_PRAGMA_WARNING_POP
         return 1;
     }
 
@@ -488,7 +491,10 @@ class TIFFInput final : public ImageInput {
     {
         TIFFInput* self = (TIFFInput*)user_data;
         spin_lock lock(self->m_last_error_mutex);
+        OIIO_PRAGMA_WARNING_PUSH
+        OIIO_GCC_PRAGMA(GCC diagnostic ignored "-Wformat-nonliteral")
         self->m_last_error = Strutil::vsprintf(fmt, ap);
+        OIIO_PRAGMA_WARNING_POP
         return 1;
     }
 #endif
@@ -534,7 +540,10 @@ oiio_tiff_last_error()
 static void
 my_error_handler(const char* /*str*/, const char* format, va_list ap)
 {
+    OIIO_PRAGMA_WARNING_PUSH
+    OIIO_GCC_PRAGMA(GCC diagnostic ignored "-Wformat-nonliteral")
     oiio_tiff_last_error() = Strutil::vsprintf(format, ap);
+    OIIO_PRAGMA_WARNING_POP
 }
 
 

src/tiff.imageio/tiffoutput.cpp

@@ -231,7 +231,10 @@ class TIFFOutput final : public ImageOutput {
     {
         TIFFOutput* self = (TIFFOutput*)user_data;
         spin_lock lock(self->m_last_error_mutex);
+        OIIO_PRAGMA_WARNING_PUSH
+        OIIO_GCC_PRAGMA(GCC diagnostic ignored "-Wformat-nonliteral")
         self->m_last_error = Strutil::vsprintf(fmt, ap);
+        OIIO_PRAGMA_WARNING_POP
         return 1;
     }
 
@@ -241,7 +244,10 @@ class TIFFOutput final : public ImageOutput {
     {
         TIFFOutput* self = (TIFFOutput*)user_data;
         spin_lock lock(self->m_last_error_mutex);
+        OIIO_PRAGMA_WARNING_PUSH
+        OIIO_GCC_PRAGMA(GCC diagnostic ignored "-Wformat-nonliteral")
         self->m_last_error = Strutil::vsprintf(fmt, ap);
+        OIIO_PRAGMA_WARNING_POP
         return 1;
     }
 #endif