From 5f1a597369272245fecd22418bd176c25f9db7a6 Mon Sep 17 00:00:00 2001 From: Shubham Mittal Date: Fri, 24 Jan 2025 15:57:29 -0800 Subject: [PATCH 1/6] added unsigned int sanitization --- CMakeLists.txt | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 61b026ead0..3db1bbf5b7 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -710,15 +710,14 @@ if(UBSAN) message(FATAL_ERROR "Cannot enable UBSAN unless using Clang") endif() - set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=undefined") - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=undefined") - set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=undefined") - + set(SANITIZER_FLAGS "-fsanitize=undefined,unsigned-integer-overflow") if(NOT UBSAN_RECOVER) - set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fno-sanitize-recover=undefined") - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-sanitize-recover=undefined") - set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fno-sanitize-recover=undefined") + set(SANITIZER_FLAGS "${SANITIZER_FLAGS} -fno-sanitize-recover=undefined,unsigned-integer-overflow") endif() + + set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${SANITIZER_FLAGS}") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${SANITIZER_FLAGS}") + set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${SANITIZER_FLAGS}") endif() if(GCOV) From d32908f8e56c75ab610c7b0eee12f20fc5aa172a Mon Sep 17 00:00:00 2001 From: Shubham Mittal Date: Mon, 27 Jan 2025 12:42:31 -0800 Subject: [PATCH 2/6] modified unsigned int options in the repo --- CMakeLists.txt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 3db1bbf5b7..6a2742871a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -709,10 +709,12 @@ if(UBSAN) if(NOT CLANG) message(FATAL_ERROR "Cannot enable UBSAN unless using Clang") endif() - - set(SANITIZER_FLAGS "-fsanitize=undefined,unsigned-integer-overflow") + # https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#id6 + # -fsanitize=undefined includes various other checks but not unsigned-integer-overflow. + # fsanitize-undefined-ignore-overflow-pattern ignores common overflow patterns that are intentional. + set(SANITIZER_FLAGS "-fsanitize=undefined,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=all") if(NOT UBSAN_RECOVER) - set(SANITIZER_FLAGS "${SANITIZER_FLAGS} -fno-sanitize-recover=undefined,unsigned-integer-overflow") + set(SANITIZER_FLAGS "${SANITIZER_FLAGS} -fno-sanitize-recover=undefined,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=all") endif() set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${SANITIZER_FLAGS}") From e3f1c10986184b9c872723eabf2ec104be33398a Mon Sep 17 00:00:00 2001 From: Shubham Mittal Date: Mon, 27 Jan 2025 16:06:48 -0800 Subject: [PATCH 3/6] narrowed down exclusions --- CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6a2742871a..6f8d55a13a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -712,9 +712,9 @@ if(UBSAN) # https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#id6 # -fsanitize=undefined includes various other checks but not unsigned-integer-overflow. # fsanitize-undefined-ignore-overflow-pattern ignores common overflow patterns that are intentional. - set(SANITIZER_FLAGS "-fsanitize=undefined,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=all") + set(SANITIZER_FLAGS "-fsanitize=undefined,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=unsigned-post-decr-while") if(NOT UBSAN_RECOVER) - set(SANITIZER_FLAGS "${SANITIZER_FLAGS} -fno-sanitize-recover=undefined,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=all") + set(SANITIZER_FLAGS "${SANITIZER_FLAGS} -fno-sanitize-recover=undefined,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=unsigned-post-decr-while") endif() set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${SANITIZER_FLAGS}") From caac874e436baadab6bb323e9930c4ecb1659179 Mon Sep 17 00:00:00 2001 From: Shubham Mittal Date: Mon, 27 Jan 2025 17:09:57 -0800 Subject: [PATCH 4/6] explicitly block sanitization in intentional spots --- CMakeLists.txt | 4 ++-- crypto/base64/base64.c | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6f8d55a13a..93953b5598 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -712,9 +712,9 @@ if(UBSAN) # https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html#id6 # -fsanitize=undefined includes various other checks but not unsigned-integer-overflow. # fsanitize-undefined-ignore-overflow-pattern ignores common overflow patterns that are intentional. - set(SANITIZER_FLAGS "-fsanitize=undefined,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=unsigned-post-decr-while") + set(SANITIZER_FLAGS "-fsanitize=undefined,unsigned-integer-overflow") if(NOT UBSAN_RECOVER) - set(SANITIZER_FLAGS "${SANITIZER_FLAGS} -fno-sanitize-recover=undefined,unsigned-integer-overflow -fsanitize-undefined-ignore-overflow-pattern=unsigned-post-decr-while") + set(SANITIZER_FLAGS "${SANITIZER_FLAGS} -fno-sanitize-recover=undefined,unsigned-integer-overflow") endif() set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${SANITIZER_FLAGS}") diff --git a/crypto/base64/base64.c b/crypto/base64/base64.c index b3a3868412..73d282ed05 100644 --- a/crypto/base64/base64.c +++ b/crypto/base64/base64.c @@ -67,6 +67,7 @@ // constant_time_lt_args_8 behaves like |constant_time_lt_8| but takes |uint8_t| // arguments for a slightly simpler implementation. +__attribute__((no_sanitize("unsigned-integer-overflow"))) static inline uint8_t constant_time_lt_args_8(uint8_t a, uint8_t b) { crypto_word_t aw = a; crypto_word_t bw = b; From c951f1ee64d0eb56f3cefbc65c5652832e53ee50 Mon Sep 17 00:00:00 2001 From: Shubham Mittal Date: Mon, 27 Jan 2025 17:21:22 -0800 Subject: [PATCH 5/6] clang specific macro --- crypto/base64/base64.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crypto/base64/base64.c b/crypto/base64/base64.c index 73d282ed05..5d0342df2f 100644 --- a/crypto/base64/base64.c +++ b/crypto/base64/base64.c @@ -67,7 +67,9 @@ // constant_time_lt_args_8 behaves like |constant_time_lt_8| but takes |uint8_t| // arguments for a slightly simpler implementation. +#if defined(__clang__) __attribute__((no_sanitize("unsigned-integer-overflow"))) +#endif static inline uint8_t constant_time_lt_args_8(uint8_t a, uint8_t b) { crypto_word_t aw = a; crypto_word_t bw = b; From e59eb6bc809b79f21b675a336b4b6b817f27e7a0 Mon Sep 17 00:00:00 2001 From: Shubham Mittal Date: Tue, 28 Jan 2025 14:45:28 -0800 Subject: [PATCH 6/6] suppress erroneous outputs --- crypto/base64/base64.c | 4 +--- crypto/bytestring/cbb.c | 1 + crypto/internal.h | 6 ++++++ ssl/internal.h | 6 ++++++ ssl/ssl_buffer.cc | 1 + 5 files changed, 15 insertions(+), 3 deletions(-) diff --git a/crypto/base64/base64.c b/crypto/base64/base64.c index 5d0342df2f..7753dd0bfe 100644 --- a/crypto/base64/base64.c +++ b/crypto/base64/base64.c @@ -67,9 +67,7 @@ // constant_time_lt_args_8 behaves like |constant_time_lt_8| but takes |uint8_t| // arguments for a slightly simpler implementation. -#if defined(__clang__) -__attribute__((no_sanitize("unsigned-integer-overflow"))) -#endif +SUPPRESS_UNSIGNED_OVERFLOW static inline uint8_t constant_time_lt_args_8(uint8_t a, uint8_t b) { crypto_word_t aw = a; crypto_word_t bw = b; diff --git a/crypto/bytestring/cbb.c b/crypto/bytestring/cbb.c index 94d89735a9..12adfcb736 100644 --- a/crypto/bytestring/cbb.c +++ b/crypto/bytestring/cbb.c @@ -430,6 +430,7 @@ int CBB_did_write(CBB *cbb, size_t len) { return 1; } +SUPPRESS_UNSIGNED_OVERFLOW static int cbb_add_u(CBB *cbb, uint64_t v, size_t len_len) { uint8_t *buf; if (!CBB_add_space(cbb, &buf, len_len)) { diff --git a/crypto/internal.h b/crypto/internal.h index 891da2931d..a5a4f152bb 100644 --- a/crypto/internal.h +++ b/crypto/internal.h @@ -234,6 +234,11 @@ typedef __uint128_t uint128_t; #define OPENSSL_HAS_BUILTIN(x) 0 #endif +#if defined(__clang__) +#define SUPPRESS_UNSIGNED_OVERFLOW __attribute__((no_sanitize("unsigned-integer-overflow"))) +#else +#define SUPPRESS_UNSIGNED_OVERFLOW +#endif // Pointer utility functions. @@ -339,6 +344,7 @@ static inline uint64_t value_barrier_u64(uint64_t a) { // constant_time_msb_w returns the given value with the MSB copied to all the // other bits. +SUPPRESS_UNSIGNED_OVERFLOW static inline crypto_word_t constant_time_msb_w(crypto_word_t a) { return 0u - (a >> (sizeof(a) * 8 - 1)); } diff --git a/ssl/internal.h b/ssl/internal.h index 7e3b21fd57..3c1c95ee25 100644 --- a/ssl/internal.h +++ b/ssl/internal.h @@ -3720,6 +3720,12 @@ struct ssl_method_st { const bssl::SSL_X509_METHOD *x509_method; }; +#if defined(__clang__) +#define SUPPRESS_UNSIGNED_OVERFLOW __attribute__((no_sanitize("unsigned-integer-overflow"))) +#else +#define SUPPRESS_UNSIGNED_OVERFLOW +#endif + #define MIN_SAFE_FRAGMENT_SIZE 512 struct ssl_ctx_st : public bssl::RefCounted { explicit ssl_ctx_st(const SSL_METHOD *ssl_method); diff --git a/ssl/ssl_buffer.cc b/ssl/ssl_buffer.cc index b61e1ad846..25f09683c2 100644 --- a/ssl/ssl_buffer.cc +++ b/ssl/ssl_buffer.cc @@ -48,6 +48,7 @@ void SSLBuffer::Clear() { buf_size_ = 0; } +SUPPRESS_UNSIGNED_OVERFLOW bool SSLBuffer::EnsureCap(size_t header_len, size_t new_cap) { if (new_cap > SSLBUFFER_MAX_CAPACITY) { OPENSSL_PUT_ERROR(SSL, ERR_R_INTERNAL_ERROR);