Skip to content

Commit 6866a69

Browse files
rustyconoverrustyconover
committed
fix: improve error checking
1 parent a97126c commit 6866a69

File tree

3 files changed

+56
-81
lines changed

3 files changed

+56
-81
lines changed

‎src/crypto_extension.cpp‎

Lines changed: 11 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -125,23 +125,6 @@ namespace duckdb
125125
throw InvalidInputException("Unsupported child type in list for crypto_hash");
126126
}
127127
}
128-
129-
// Helper to lookup algorithm - returns nullptr for blake3, throws on invalid algorithm
130-
const EVP_MD *LookupAlgorithm(const std::string &hash_name_str)
131-
{
132-
if (hash_name_str == "blake3")
133-
{
134-
return nullptr;
135-
}
136-
137-
auto it = GetDigestMap().find(hash_name_str);
138-
if (it != GetDigestMap().end())
139-
{
140-
return it->second();
141-
}
142-
143-
throw InvalidInputException("Invalid hash algorithm '" + hash_name_str + "'");
144-
}
145128
}
146129

147130
inline void CryptoScalarHashFun(DataChunk &args, ExpressionState &state, Vector &result)
@@ -325,25 +308,13 @@ namespace duckdb
325308

326309
int64_t length = lengths[length_idx];
327310

328-
// Validate length before allocation (CryptoRandomBytes will validate too, but we need to prevent allocation issues)
329-
if (length <= 0)
330-
{
331-
throw InvalidInputException("Random bytes length must be greater than 0");
332-
}
333-
334-
// DuckDB BLOB maximum size is 4GB (2^32 - 1 bytes)
335-
constexpr int64_t MAX_BLOB_SIZE = 4294967295LL; // 4GB - 1
336-
if (length > MAX_BLOB_SIZE)
337-
{
338-
throw InvalidInputException(
339-
"Random bytes length must be less than or equal to " +
340-
std::to_string(MAX_BLOB_SIZE) + " bytes (4GB)");
341-
}
311+
// Validate length before allocation to prevent allocation issues
312+
ValidateRandomBytesLength(length);
342313

343314
// Allocate buffer for random bytes
344315
auto buffer = std::unique_ptr<unsigned char[]>(new unsigned char[length]);
345316

346-
// Generate random bytes (will also validate length)
317+
// Generate random bytes
347318
CryptoRandomBytes(length, buffer.get());
348319

349320
// Add result as BLOB
@@ -505,7 +476,14 @@ namespace duckdb
505476
else
506477
{
507478
target.ctx = EVP_MD_CTX_new();
508-
EVP_MD_CTX_copy_ex(target.ctx, source.ctx);
479+
if (target.ctx == nullptr)
480+
{
481+
throw InternalException("Failed to create hash context");
482+
}
483+
if (EVP_MD_CTX_copy_ex(target.ctx, source.ctx) != 1)
484+
{
485+
throw InternalException("Failed to copy hash context");
486+
}
509487
}
510488
target.is_touched = true;
511489
}

‎src/crypto_hash.cpp‎

Lines changed: 36 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -45,32 +45,50 @@ namespace duckdb
4545
}
4646
return result;
4747
}
48+
}
49+
50+
const EVP_MD *LookupAlgorithm(const std::string &algorithm)
51+
{
52+
std::string algo_lower = StringUtil::Lower(algorithm);
4853

49-
const EVP_MD *getDigestByName(const std::string &algorithm)
54+
if (algo_lower == "blake3")
5055
{
51-
std::string algo_lower = StringUtil::Lower(algorithm);
56+
return nullptr;
57+
}
5258

53-
if (algo_lower == "blake3")
54-
{
55-
return nullptr;
56-
}
59+
auto it = GetDigestMap().find(algo_lower);
60+
if (it != GetDigestMap().end())
61+
{
62+
return it->second();
63+
}
5764

58-
auto it = GetDigestMap().find(algo_lower);
59-
if (it != GetDigestMap().end())
60-
{
61-
return it->second();
62-
}
65+
throw InvalidInputException(
66+
"Invalid hash algorithm '" + algorithm + "'. " +
67+
"Available algorithms are: " + getAvailableAlgorithms());
68+
}
6369

64-
return nullptr;
70+
void ValidateRandomBytesLength(int64_t length)
71+
{
72+
if (length <= 0)
73+
{
74+
throw InvalidInputException("Random bytes length must be greater than 0");
75+
}
76+
77+
if (length > CRYPTO_MAX_RANDOM_BYTES)
78+
{
79+
throw InvalidInputException(
80+
"Random bytes length must be less than or equal to " +
81+
std::to_string(CRYPTO_MAX_RANDOM_BYTES) + " bytes (4GB)");
6582
}
6683
}
6784

6885
void CryptoHash(const std::string &algorithm, const char *data, size_t data_len, unsigned char *result, unsigned int &result_len)
6986
{
70-
std::string algo_lower = StringUtil::Lower(algorithm);
87+
// LookupAlgorithm returns nullptr for blake3, throws on invalid algorithm
88+
const EVP_MD *md = LookupAlgorithm(algorithm);
7189

7290
// Handle Blake3 separately since it doesn't use OpenSSL
73-
if (algo_lower == "blake3")
91+
if (md == nullptr)
7492
{
7593
blake3_hasher hasher;
7694
blake3_hasher_init(&hasher);
@@ -80,15 +98,6 @@ namespace duckdb
8098
return;
8199
}
82100

83-
const EVP_MD *md = getDigestByName(algo_lower);
84-
85-
if (md == nullptr)
86-
{
87-
throw InvalidInputException(
88-
"Invalid hash algorithm '" + algorithm + "'. " +
89-
"Available algorithms are: " + getAvailableAlgorithms());
90-
}
91-
92101
EVP_MD_CTX *ctx = EVP_MD_CTX_new();
93102
if (ctx == nullptr)
94103
{
@@ -113,10 +122,11 @@ namespace duckdb
113122

114123
void CryptoHmac(const std::string &algorithm, const std::string &key, const std::string &data, unsigned char *result, unsigned int &result_len)
115124
{
116-
std::string algo_lower = StringUtil::Lower(algorithm);
125+
// LookupAlgorithm returns nullptr for blake3, throws on invalid algorithm
126+
const EVP_MD *md = LookupAlgorithm(algorithm);
117127

118128
// Handle Blake3 HMAC separately
119-
if (algo_lower == "blake3")
129+
if (md == nullptr)
120130
{
121131
// Blake3 keyed mode requires exactly 32 bytes for the key
122132
if (key.size() != BLAKE3_KEY_LEN)
@@ -132,15 +142,6 @@ namespace duckdb
132142
return;
133143
}
134144

135-
const EVP_MD *md = getDigestByName(algo_lower);
136-
137-
if (md == nullptr)
138-
{
139-
throw InvalidInputException(
140-
"Invalid hash algorithm '" + algorithm + "'. " +
141-
"Available algorithms are: " + getAvailableAlgorithms());
142-
}
143-
144145
unsigned char *hmac_result = HMAC(
145146
md,
146147
key.data(), key.size(),
@@ -155,20 +156,7 @@ namespace duckdb
155156

156157
void CryptoRandomBytes(int64_t length, unsigned char *result)
157158
{
158-
// Validate input length
159-
if (length <= 0)
160-
{
161-
throw InvalidInputException("Random bytes length must be greater than 0");
162-
}
163-
164-
// DuckDB BLOB maximum size is 4GB (2^32 - 1 bytes)
165-
constexpr int64_t MAX_BLOB_SIZE = 4294967295LL; // 4GB - 1
166-
if (length > MAX_BLOB_SIZE)
167-
{
168-
throw InvalidInputException(
169-
"Random bytes length must be less than or equal to " +
170-
std::to_string(MAX_BLOB_SIZE) + " bytes (4GB)");
171-
}
159+
ValidateRandomBytesLength(length);
172160

173161
// Generate random bytes using OpenSSL's RAND_bytes
174162
// RAND_bytes is cryptographically secure and automatically seeds itself

‎src/include/crypto_hash.hpp‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,18 @@ typedef struct evp_md_st EVP_MD;
1010

1111
namespace duckdb {
1212

13+
// Maximum size for random bytes (DuckDB BLOB max is 4GB - 1)
14+
constexpr int64_t CRYPTO_MAX_RANDOM_BYTES = 4294967295LL;
15+
1316
// Get the shared digest map for algorithm name lookups
1417
const std::unordered_map<std::string, std::function<const EVP_MD *()>>& GetDigestMap();
1518

19+
// Lookup algorithm by name - returns nullptr for blake3, throws on invalid algorithm
20+
const EVP_MD* LookupAlgorithm(const std::string& algorithm);
21+
22+
// Validate random bytes length - throws InvalidInputException if invalid
23+
void ValidateRandomBytesLength(int64_t length);
24+
1625
// Compute a cryptographic hash of the input data
1726
void CryptoHash(const std::string& algorithm, const std::string& data, unsigned char* result, unsigned int& result_len);
1827

0 commit comments

Comments
 (0)