Skip to content

Commit

Permalink
cipher: fix buffer overflow in Cipher#update
Browse files Browse the repository at this point in the history
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 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 way to tell the maximum
required buffer size for arbitrary ciphers, but this is large enough for
all algorithms implemented in current versions of OpenSSL.

Fixes: https://bugs.ruby-lang.org/issues/20236
  • Loading branch information
rhenium committed Feb 5, 2024
1 parent ccc1594 commit 67441ba
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 3 deletions.
18 changes: 15 additions & 3 deletions ext/openssl/ossl_cipher.c
Original file line number Diff line number Diff line change
Expand Up @@ -387,11 +387,23 @@ ossl_cipher_update(int argc, VALUE *argv, VALUE self)
if ((in_len = RSTRING_LEN(data)) == 0)
ossl_raise(rb_eArgError, "data must not be empty");
GetCipher(self, ctx);
out_len = in_len+EVP_CIPHER_CTX_block_size(ctx);
if (out_len <= 0) {

/*
* As of OpenSSL 3.2, there is no reliable way to determine the required
* output buffer size for arbitrary cipher modes.
* https://github.com/openssl/openssl/issues/22628
*
* in_len+block_size is usually sufficient, but AES key wrap with padding
* ciphers require in_len+15 even though they have a block size of 8 bytes.
*
* Using EVP_MAX_BLOCK_LENGTH (32) as a safe upper bound for ciphers
* currently implemented in OpenSSL, but this can change in the future.
*/
if (in_len > LONG_MAX - EVP_MAX_BLOCK_LENGTH) {
ossl_raise(rb_eRangeError,
"data too big to make output buffer: %ld bytes", in_len);
}
out_len = in_len + EVP_MAX_BLOCK_LENGTH;

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

if (!ossl_cipher_update_long(ctx, (unsigned char *)RSTRING_PTR(str), &out_len, in, in_len))
ossl_raise(eCipherError, NULL);
assert(out_len < RSTRING_LEN(str));
assert(out_len <= RSTRING_LEN(str));
rb_str_set_len(str, out_len);

return str;
Expand Down
12 changes: 12 additions & 0 deletions test/openssl/test_cipher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,18 @@ def test_crypt_after_key
end
end

def test_aes_keywrap_pad
# RFC 5649 Section 6; The second example
kek = ["5840df6e29b02af1ab493b705bf16ea1ae8338f4dcc176a8"].pack("H*")
key = ["466f7250617369"].pack("H*")
wrap = ["afbeb0f07dfbf5419200f2ccb50bb24f"].pack("H*")

cipher = OpenSSL::Cipher.new("aes192-wrap-pad").encrypt
cipher.key = kek
ct = cipher.update(key) << cipher.final
assert_equal wrap, ct
end if has_cipher?("aes192-wrap-pad")

private

def new_encryptor(algo, **kwargs)
Expand Down

0 comments on commit 67441ba

Please sign in to comment.