Skip to content

Commit d586ed1

Browse files
authored
GH-49752: [C++][Gandiva] Fix potential buffer overrun in Gandiva SSL function (#49780)
### Rationale for this change Fixes security related problems found in gdv_hash_using_openssl. Those problems were not deemed to be a security risk. ### What changes are included in this PR? **[hash_utils.h:41, hash_utils.cc:66]** Removed GANDIVA_EXPORT from gdv_hash_using_openssl — it's an internal helper, not part of the public API. **[hash_utils.cc:105]** Changed && → || in the validation condition. The original only errored when both checks failed; now it errors when either result_length != hash_digest_size or result_buf_size != (2 * hash_digest_size). **[hash_utils.cc:135]** Fixed snprintf buffer size, so it correctly accounts for the already-written bytes and prevents potential out-of-bounds writes. Allocate result_buf_size + 1 bytes — the extra byte absorbs the final null terminator. Pass result_buf_size - result_buff_index + 1 to snprintf — reflects the actual remaining space (2 hex chars + 1 null = 3 bytes on the last call), preventing any potential overflow if the format ever changed. ### Are these changes tested? Yes, unit tests. ### Are there any user-facing changes? No. * GitHub Issue: #49752 Authored-by: logan.riggs@gmail.com <logan.riggs@gmail.com> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
1 parent e2b4437 commit d586ed1

2 files changed

Lines changed: 48 additions & 49 deletions

File tree

‎cpp/src/gandiva/hash_utils.cc‎

Lines changed: 48 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -21,49 +21,12 @@
2121
#include "gandiva/gdv_function_stubs.h"
2222
#include "openssl/evp.h"
2323

24-
namespace gandiva {
25-
26-
/// Hashes a generic message using the SHA512 algorithm
27-
GANDIVA_EXPORT
28-
const char* gdv_sha512_hash(int64_t context, const void* message, size_t message_length,
29-
int32_t* out_length) {
30-
constexpr int sha512_result_length = 128;
31-
return gdv_hash_using_openssl(context, message, message_length, EVP_sha512(),
32-
sha512_result_length, out_length);
33-
}
34-
35-
/// Hashes a generic message using the SHA256 algorithm
36-
GANDIVA_EXPORT
37-
const char* gdv_sha256_hash(int64_t context, const void* message, size_t message_length,
38-
int32_t* out_length) {
39-
constexpr int sha256_result_length = 64;
40-
return gdv_hash_using_openssl(context, message, message_length, EVP_sha256(),
41-
sha256_result_length, out_length);
42-
}
43-
44-
/// Hashes a generic message using the SHA1 algorithm
45-
GANDIVA_EXPORT
46-
const char* gdv_sha1_hash(int64_t context, const void* message, size_t message_length,
47-
int32_t* out_length) {
48-
constexpr int sha1_result_length = 40;
49-
return gdv_hash_using_openssl(context, message, message_length, EVP_sha1(),
50-
sha1_result_length, out_length);
51-
}
52-
53-
GANDIVA_EXPORT
54-
const char* gdv_md5_hash(int64_t context, const void* message, size_t message_length,
55-
int32_t* out_length) {
56-
constexpr int md5_result_length = 32;
57-
return gdv_hash_using_openssl(context, message, message_length, EVP_md5(),
58-
md5_result_length, out_length);
59-
}
60-
61-
/// \brief Hashes a generic message using SHA algorithm.
24+
namespace {
25+
/// \brief Hashes a generic message using a hash/digest algorithm.
6226
///
6327
/// It uses the EVP API in the OpenSSL library to generate
6428
/// the hash. The type of the hash is defined by the
6529
/// \b hash_type \b parameter.
66-
GANDIVA_EXPORT
6730
const char* gdv_hash_using_openssl(int64_t context, const void* message,
6831
size_t message_length, const EVP_MD* hash_type,
6932
uint32_t result_buf_size, int32_t* out_length) {
@@ -100,9 +63,10 @@ const char* gdv_hash_using_openssl(int64_t context, const void* message,
10063
}
10164

10265
unsigned int result_length;
103-
EVP_DigestFinal_ex(md_ctx, result, &result_length);
66+
int evp_result = EVP_DigestFinal_ex(md_ctx, result, &result_length);
10467

105-
if (result_length != hash_digest_size && result_buf_size != (2 * hash_digest_size)) {
68+
if (evp_result != evp_success_status || result_length != hash_digest_size ||
69+
result_buf_size != (2 * hash_digest_size)) {
10670
gdv_fn_context_set_error_msg(context,
10771
"Could not obtain the hash for the defined value");
10872
EVP_MD_CTX_free(md_ctx);
@@ -112,8 +76,10 @@ const char* gdv_hash_using_openssl(int64_t context, const void* message,
11276
return "";
11377
}
11478

79+
// Allocate one extra byte beyond result_buf_size so that the null terminator
80+
// written by the final snprintf call does not land past the end of the buffer.
11581
auto result_buffer =
116-
reinterpret_cast<char*>(gdv_fn_context_arena_malloc(context, result_buf_size));
82+
reinterpret_cast<char*>(gdv_fn_context_arena_malloc(context, result_buf_size + 1));
11783

11884
if (result_buffer == nullptr) {
11985
gdv_fn_context_set_error_msg(context,
@@ -132,7 +98,8 @@ const char* gdv_hash_using_openssl(int64_t context, const void* message,
13298

13399
unsigned char hex_number = result[j];
134100
result_buff_index +=
135-
snprintf(result_buffer + result_buff_index, result_buf_size, "%02x", hex_number);
101+
snprintf(result_buffer + result_buff_index,
102+
result_buf_size - result_buff_index + 1, "%02x", hex_number);
136103
}
137104

138105
// Free the resources used by the EVP to avoid memory leaks
@@ -142,6 +109,44 @@ const char* gdv_hash_using_openssl(int64_t context, const void* message,
142109
*out_length = result_buf_size;
143110
return result_buffer;
144111
}
112+
} // namespace
113+
114+
namespace gandiva {
115+
116+
/// Hashes a generic message using the SHA512 algorithm
117+
GANDIVA_EXPORT
118+
const char* gdv_sha512_hash(int64_t context, const void* message, size_t message_length,
119+
int32_t* out_length) {
120+
constexpr int sha512_result_length = 128;
121+
return gdv_hash_using_openssl(context, message, message_length, EVP_sha512(),
122+
sha512_result_length, out_length);
123+
}
124+
125+
/// Hashes a generic message using the SHA256 algorithm
126+
GANDIVA_EXPORT
127+
const char* gdv_sha256_hash(int64_t context, const void* message, size_t message_length,
128+
int32_t* out_length) {
129+
constexpr int sha256_result_length = 64;
130+
return gdv_hash_using_openssl(context, message, message_length, EVP_sha256(),
131+
sha256_result_length, out_length);
132+
}
133+
134+
/// Hashes a generic message using the SHA1 algorithm
135+
GANDIVA_EXPORT
136+
const char* gdv_sha1_hash(int64_t context, const void* message, size_t message_length,
137+
int32_t* out_length) {
138+
constexpr int sha1_result_length = 40;
139+
return gdv_hash_using_openssl(context, message, message_length, EVP_sha1(),
140+
sha1_result_length, out_length);
141+
}
142+
143+
GANDIVA_EXPORT
144+
const char* gdv_md5_hash(int64_t context, const void* message, size_t message_length,
145+
int32_t* out_length) {
146+
constexpr int md5_result_length = 32;
147+
return gdv_hash_using_openssl(context, message, message_length, EVP_md5(),
148+
md5_result_length, out_length);
149+
}
145150

146151
GANDIVA_EXPORT
147152
uint64_t gdv_double_to_long(double value) {

‎cpp/src/gandiva/hash_utils.h‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
#include <cstdint>
2222
#include <cstdlib>
2323
#include "gandiva/visibility.h"
24-
#include "openssl/evp.h"
2524

2625
namespace gandiva {
2726

@@ -37,11 +36,6 @@ GANDIVA_EXPORT
3736
const char* gdv_sha1_hash(int64_t context, const void* message, size_t message_length,
3837
int32_t* out_length);
3938

40-
GANDIVA_EXPORT
41-
const char* gdv_hash_using_openssl(int64_t context, const void* message,
42-
size_t message_length, const EVP_MD* hash_type,
43-
uint32_t result_buf_size, int32_t* out_length);
44-
4539
GANDIVA_EXPORT
4640
const char* gdv_md5_hash(int64_t context, const void* message, size_t message_length,
4741
int32_t* out_length);

0 commit comments

Comments
 (0)