Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions cpp/src/gandiva/hash_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ const char* gdv_md5_hash(int64_t context, const void* message, size_t message_le
/// It uses the EVP API in the OpenSSL library to generate
/// the hash. The type of the hash is defined by the
/// \b hash_type \b parameter.
Comment on lines 25 to 29
GANDIVA_EXPORT
Comment thread
lriggs marked this conversation as resolved.
const char* gdv_hash_using_openssl(int64_t context, const void* message,
size_t message_length, const EVP_MD* hash_type,
uint32_t result_buf_size, int32_t* out_length) {
Expand Down Expand Up @@ -102,7 +101,7 @@ const char* gdv_hash_using_openssl(int64_t context, const void* message,
unsigned int result_length;
EVP_DigestFinal_ex(md_ctx, result, &result_length);

Comment thread
lriggs marked this conversation as resolved.
if (result_length != hash_digest_size && result_buf_size != (2 * hash_digest_size)) {
if (result_length != hash_digest_size || result_buf_size != (2 * hash_digest_size)) {
gdv_fn_context_set_error_msg(context,
"Could not obtain the hash for the defined value");
EVP_MD_CTX_free(md_ctx);
Expand All @@ -112,8 +111,10 @@ const char* gdv_hash_using_openssl(int64_t context, const void* message,
return "";
}

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

if (result_buffer == nullptr) {
gdv_fn_context_set_error_msg(context,
Expand All @@ -132,7 +133,8 @@ const char* gdv_hash_using_openssl(int64_t context, const void* message,

unsigned char hex_number = result[j];
result_buff_index +=
snprintf(result_buffer + result_buff_index, result_buf_size, "%02x", hex_number);
snprintf(result_buffer + result_buff_index,
result_buf_size - result_buff_index + 1, "%02x", hex_number);
}

// Free the resources used by the EVP to avoid memory leaks
Expand Down
1 change: 0 additions & 1 deletion cpp/src/gandiva/hash_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ GANDIVA_EXPORT
const char* gdv_sha1_hash(int64_t context, const void* message, size_t message_length,
int32_t* out_length);

GANDIVA_EXPORT
const char* gdv_hash_using_openssl(int64_t context, const void* message,
size_t message_length, const EVP_MD* hash_type,
uint32_t result_buf_size, int32_t* out_length);
Comment thread
lriggs marked this conversation as resolved.
Outdated
Expand Down
20 changes: 20 additions & 0 deletions cpp/src/gandiva/hash_utils_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

#include "gandiva/execution_context.h"
#include "gandiva/hash_utils.h"
#include "openssl/evp.h"
Comment thread
lriggs marked this conversation as resolved.
Outdated
Comment thread
lriggs marked this conversation as resolved.
Outdated

TEST(TestShaHashUtils, TestSha1Numeric) {
gandiva::ExecutionContext ctx;
Expand Down Expand Up @@ -316,3 +317,22 @@ TEST(TestShaHashUtils, TestMD5Varlen) {
EXPECT_EQ(md5_2_as_str.size(), md5_size);
EXPECT_EQ(md5_2_as_str, expected_second_result);
}

// Verify that gdv_hash_using_openssl() reports an error when result_buf_size does not
// equal 2 * hash_digest_size (tests the || fix from GH-49752).
TEST(TestShaHashUtils, TestHashUsingOpenSSLInvalidBufSize) {
gandiva::ExecutionContext ctx;
auto ctx_ptr = reinterpret_cast<int64_t>(&ctx);

std::string msg = "hello";
int32_t out_length = -1;

// SHA-256 digest is 32 bytes, so result_buf_size must be 64. Pass 63 to trigger
// the error path that was previously guarded by && instead of ||.
const char* result = gandiva::gdv_hash_using_openssl(
Comment thread
lriggs marked this conversation as resolved.
Outdated
ctx_ptr, msg.c_str(), msg.size(), EVP_sha256(), /*result_buf_size=*/63, &out_length);

EXPECT_TRUE(ctx.has_error());
EXPECT_EQ(out_length, 0);
EXPECT_STREQ(result, "");
}
Loading