Skip to content

Commit 3035559

Browse files
committed
cipher: fix buffer overflow in Cipher#update
OpenSSL::Cipher#update currently allocates the output buffer with size (input data length)+(the block size of the cipher). This is insufficient for the id-aes{128,192,256}-wrap-pad (AES keywrap with padding) ciphers. They have a block size of 8 bytes, but the output may be up to 15 bytes larger than the input. Use (input data length)+EVP_MAX_BLOCK_LENGTH (== 32) as the output buffer size, instead. OpenSSL doesn't provide a generic way to tell the maximum required buffer size for ciphers, but this is large enough for all algorithms implemented in current versions of OpenSSL. Fixes: https://bugs.ruby-lang.org/issues/20236
1 parent 3bdde7d commit 3035559

File tree

2 files changed

+31
-3
lines changed

2 files changed

+31
-3
lines changed

ext/openssl/ossl_cipher.c

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -387,11 +387,23 @@ ossl_cipher_update(int argc, VALUE *argv, VALUE self)
387387
if ((in_len = RSTRING_LEN(data)) == 0)
388388
ossl_raise(rb_eArgError, "data must not be empty");
389389
GetCipher(self, ctx);
390-
out_len = in_len+EVP_CIPHER_CTX_block_size(ctx);
391-
if (out_len <= 0) {
390+
391+
/*
392+
* As of OpenSSL 3.2, there is no reliable way to determine the required
393+
* output buffer size for arbitrary cipher modes.
394+
* https://github.com/openssl/openssl/issues/22628
395+
*
396+
* in_len+block_size is usually sufficient, but AES key wrap with padding
397+
* ciphers require in_len+15 even though they have a block size of 8 bytes.
398+
*
399+
* Using EVP_MAX_BLOCK_LENGTH (32) as a safe upper bound for ciphers
400+
* currently implemented in OpenSSL, but this can change in the future.
401+
*/
402+
if (in_len > LONG_MAX - EVP_MAX_BLOCK_LENGTH) {
392403
ossl_raise(rb_eRangeError,
393404
"data too big to make output buffer: %ld bytes", in_len);
394405
}
406+
out_len = in_len + EVP_MAX_BLOCK_LENGTH;
395407

396408
if (NIL_P(str)) {
397409
str = rb_str_new(0, out_len);
@@ -402,7 +414,7 @@ ossl_cipher_update(int argc, VALUE *argv, VALUE self)
402414

403415
if (!ossl_cipher_update_long(ctx, (unsigned char *)RSTRING_PTR(str), &out_len, in, in_len))
404416
ossl_raise(eCipherError, NULL);
405-
assert(out_len < RSTRING_LEN(str));
417+
assert(out_len <= RSTRING_LEN(str));
406418
rb_str_set_len(str, out_len);
407419

408420
return str;

test/openssl/test_cipher.rb

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,22 @@ def test_aes_gcm_key_iv_order_issue
337337
assert_equal tag1, tag2
338338
end
339339

340+
def test_aes_keywrap_pad
341+
# RFC 5649 Section 6; The second example
342+
kek = ["5840df6e29b02af1ab493b705bf16ea1ae8338f4dcc176a8"].pack("H*")
343+
key = ["466f7250617369"].pack("H*")
344+
wrap = ["afbeb0f07dfbf5419200f2ccb50bb24f"].pack("H*")
345+
346+
begin
347+
cipher = OpenSSL::Cipher.new("id-aes192-wrap-pad").encrypt
348+
rescue OpenSSL::Cipher::CipherError, RuntimeError
349+
omit "id-aes192-wrap-pad is not supported: #$!"
350+
end
351+
cipher.key = kek
352+
ct = cipher.update(key) << cipher.final
353+
assert_equal wrap, ct
354+
end
355+
340356
def test_non_aead_cipher_set_auth_data
341357
assert_raise(OpenSSL::Cipher::CipherError) {
342358
cipher = OpenSSL::Cipher.new("aes-128-cfb").encrypt

0 commit comments

Comments
 (0)