Skip to content

Commit 79d76f5

Browse files
committed
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]>
1 parent baf85bd commit 79d76f5

File tree

5 files changed

+82
-26
lines changed

5 files changed

+82
-26
lines changed

src/cmake/compiler.cmake

Lines changed: 60 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ elseif (CMAKE_CXX_COMPILER_ID MATCHES "Intel")
8686
set (CMAKE_COMPILER_IS_INTEL 1)
8787
message (VERBOSE "Using Intel as the compiler")
8888
endif ()
89+
if (CMAKE_COMPILER_IS_GNUCC OR CMAKE_COMPILER_IS_CLANG)
90+
set (COMPILER_IS_GCC_OR_ANY_CLANG TRUE)
91+
endif ()
8992

9093

9194
###########################################################################
@@ -102,12 +105,12 @@ else ()
102105
endif()
103106
option (EXTRA_WARNINGS "Enable lots of extra pedantic warnings" OFF)
104107
if (NOT MSVC)
105-
add_compile_options ("-Wall")
108+
add_compile_options (-Wall -Wformat -Wformat=2)
106109
if (EXTRA_WARNINGS)
107-
add_compile_options ("-Wextra")
110+
add_compile_options (-Wextra)
108111
endif ()
109112
if (STOP_ON_WARNING)
110-
add_compile_options ("-Werror")
113+
add_compile_options (-Werror)
111114
endif ()
112115
endif ()
113116

@@ -462,25 +465,60 @@ endif ()
462465

463466

464467
###########################################################################
465-
# Fortification and hardening options
466-
#
467-
# In modern gcc and clang, FORTIFY_SOURCE provides buffer overflow checks
468-
# (with some compiler-assisted deduction of buffer lengths) for the following
469-
# functions: memcpy, mempcpy, memmove, memset, strcpy, stpcpy, strncpy,
470-
# strcat, strncat, sprintf, vsprintf, snprintf, vsnprintf, gets.
471-
#
472-
# We try to avoid these unsafe functions anyway, but it's good to have the
473-
# extra protection, at least as an extra set of checks during CI. Some users
474-
# may also wish to enable it at some level if they are deploying it in a
475-
# security-sensitive environment. FORTIFY_SOURCE=3 may have minor performance
476-
# impact, though FORTIFY_SOURCE=2 should not have a measurable effect on
477-
# performance versus not doing any fortification. All fortification levels are
478-
# not available in all compilers.
479-
480-
set (FORTIFY_SOURCE "0" CACHE STRING "Turn on Fortification level (0, 1, 2, 3)")
481-
if (FORTIFY_SOURCE AND (CMAKE_COMPILER_IS_GNUCC OR CMAKE_COMPILER_IS_CLANG))
482-
message (STATUS "Compiling with _FORTIFY_SOURCE=${FORTIFY_SOURCE}")
483-
add_compile_options (-D_FORTIFY_SOURCE=${FORTIFY_SOURCE})
468+
# Safety/security hardening options
469+
#
470+
# Explanation of levels:
471+
# 0 : do nothing, not recommended.
472+
# 1 : enable features that have no (or nearly no) performance impact,
473+
# recommended default for optimized, shipping code.
474+
# 2 : enable features that trade off performance for security, recommended
475+
# for debugging or deploying in security-sensitive environments.
476+
# 3 : enable features that have a significant performance impact, only
477+
# recommended for debugging.
478+
#
479+
# Some documentation:
480+
# https://best.openssf.org/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C++.html
481+
# https://www.gnu.org/software/libc/manual/html_node/Source-Fortification.html
482+
# https://gcc.gnu.org/onlinedocs/libstdc++/manual/using_macros.html
483+
# https://libcxx.llvm.org/Hardening.html
484+
#
485+
if (${CMAKE_BUILD_TYPE} STREQUAL "Debug")
486+
set (${PROJ_NAME}_HARDENING_DEFAULT 3)
487+
else ()
488+
set (${PROJ_NAME}_HARDENING_DEFAULT 1)
489+
endif ()
490+
set_cache (${PROJ_NAME}_HARDENING ${${PROJ_NAME}_HARDENING_DEFAULT}
491+
"Turn on security hardening features 0, 1, 2, 3")
492+
# Implementation:
493+
if (HARDENING GREATER_EQUAL 1)
494+
# Features that should not detectably affect performance
495+
if (COMPILER_IS_GCC_OR_ANY_CLANG)
496+
# Enable PIE and pie to build as position-independent executables,
497+
# needed for address space randomiztion used by some kernels.
498+
add_compile_options (-fPIE -pie)
499+
add_link_options (-fPIE -pie)
500+
# Protect against stack overwrites. Is allegedly not a performance
501+
# tradeoff.
502+
add_compile_options (-fstack-protector-strong)
503+
add_link_options (-fstack-protector-strong)
504+
endif ()
505+
# Defining _FORTIFY_SOURCE provides buffer overflow checks in modern gcc &
506+
# clang with some compiler-assisted deduction of buffer lengths) for the
507+
# many C functions such as memcpy, strcpy, sprintf, etc. See:
508+
add_compile_definitions (_FORTIFY_SOURCE=${${PROJ_NAME}_HARDENING})
509+
# Setting _LIBCPP_HARDENING_MODE enables various hardening features in
510+
# clang/llvm's libc++ 18.0 and later.
511+
add_compiler_definitions (_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_FAST)
512+
endif ()
513+
if (HARDENING GREATER_EQUAL 2)
514+
# Features that might impact performance measurably
515+
add_compile_definitions (_GLIBCXX_ASSERTIONS)
516+
add_compiler_definitions (_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_EXTENSIVE)
517+
endif ()
518+
if (HARDENING GREATER_EQUAL 3)
519+
# Debugging features that might impact performance significantly
520+
add_compile_definitions (_GLIBCXX_DEBUG)
521+
add_compiler_definitions (_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG)
484522
endif ()
485523

486524

src/cmake/set_utils.cmake

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,9 @@ macro (super_set name value)
9898
set (${name} ${_sce_val} ${_sce_extra_args})
9999
endif ()
100100
if (_sce_VERBOSE)
101-
message (STATUS "${name} = ${${name}}")
101+
message (STATUS "${name} = ${${name}} (${_sce_DOC})")
102102
else ()
103-
message (VERBOSE "${name} = ${${name}}")
103+
message (VERBOSE "${name} = ${${name}} (${_sce_DOC})")
104104
endif ()
105105
if (_sce_ADVANCED)
106106
mark_as_advanced (${name})

src/libutil/SHA1.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -269,10 +269,13 @@ bool CSHA1::ReportHash(TCHAR* tszReport, REPORT_TYPE rtReportType) const
269269
_sntprintf(tszTemp, 15, _T("%02X"), m_digest[0]);
270270
Strutil::safe_strcpy(tszReport, tszTemp, HASH_TEMP_BUFFER_SIZE - 1);
271271

272-
const TCHAR* lpFmt = ((rtReportType == REPORT_HEX) ? _T(" %02X") : _T("%02X"));
272+
// const TCHAR* lpFmt = ((rtReportType == REPORT_HEX) ? _T(" %02X") : _T("%02X"));
273273
for(size_t i = 1; i < 20; ++i)
274274
{
275-
_sntprintf(tszTemp, 15, lpFmt, m_digest[i]);
275+
if (rtReportType == REPORT_HEX)
276+
_sntprintf(tszTemp, 15, _T(" %02X"), m_digest[i]);
277+
else
278+
_sntprintf(tszTemp, 15, _T("%02X"), m_digest[i]);
276279
Strutil::safe_strcat(tszReport, tszTemp, HASH_TEMP_BUFFER_SIZE - 1);
277280
}
278281
}

src/tiff.imageio/tiffinput.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,7 +478,10 @@ class TIFFInput final : public ImageInput {
478478
{
479479
TIFFInput* self = (TIFFInput*)user_data;
480480
spin_lock lock(self->m_last_error_mutex);
481+
OIIO_PRAGMA_WARNING_PUSH
482+
OIIO_GCC_PRAGMA(GCC diagnostic ignored "-Wformat-nonliteral")
481483
self->m_last_error = Strutil::vsprintf(fmt, ap);
484+
OIIO_PRAGMA_WARNING_POP
482485
return 1;
483486
}
484487

@@ -488,7 +491,10 @@ class TIFFInput final : public ImageInput {
488491
{
489492
TIFFInput* self = (TIFFInput*)user_data;
490493
spin_lock lock(self->m_last_error_mutex);
494+
OIIO_PRAGMA_WARNING_PUSH
495+
OIIO_GCC_PRAGMA(GCC diagnostic ignored "-Wformat-nonliteral")
491496
self->m_last_error = Strutil::vsprintf(fmt, ap);
497+
OIIO_PRAGMA_WARNING_POP
492498
return 1;
493499
}
494500
#endif
@@ -534,7 +540,10 @@ oiio_tiff_last_error()
534540
static void
535541
my_error_handler(const char* /*str*/, const char* format, va_list ap)
536542
{
543+
OIIO_PRAGMA_WARNING_PUSH
544+
OIIO_GCC_PRAGMA(GCC diagnostic ignored "-Wformat-nonliteral")
537545
oiio_tiff_last_error() = Strutil::vsprintf(format, ap);
546+
OIIO_PRAGMA_WARNING_POP
538547
}
539548

540549

src/tiff.imageio/tiffoutput.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,10 @@ class TIFFOutput final : public ImageOutput {
231231
{
232232
TIFFOutput* self = (TIFFOutput*)user_data;
233233
spin_lock lock(self->m_last_error_mutex);
234+
OIIO_PRAGMA_WARNING_PUSH
235+
OIIO_GCC_PRAGMA(GCC diagnostic ignored "-Wformat-nonliteral")
234236
self->m_last_error = Strutil::vsprintf(fmt, ap);
237+
OIIO_PRAGMA_WARNING_POP
235238
return 1;
236239
}
237240

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

0 commit comments

Comments
 (0)