Skip to content

Commit

Permalink
build: Add hardening options (AcademySoftwareFoundation#4538)
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
lgritz committed Nov 15, 2024
1 parent baf85bd commit 79d76f5
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 26 deletions.
82 changes: 60 additions & 22 deletions src/cmake/compiler.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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 ()


###########################################################################
Expand All @@ -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 ()

Expand Down Expand Up @@ -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 ()


Expand Down
4 changes: 2 additions & 2 deletions src/cmake/set_utils.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -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})
Expand Down
7 changes: 5 additions & 2 deletions src/libutil/SHA1.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
9 changes: 9 additions & 0 deletions src/tiff.imageio/tiffinput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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
Expand Down Expand Up @@ -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
}


Expand Down
6 changes: 6 additions & 0 deletions src/tiff.imageio/tiffoutput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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
Expand Down

0 comments on commit 79d76f5

Please sign in to comment.