Skip to content

Commit 998e2f4

Browse files
apocalypse9949matzbot
authored andcommitted
[ruby/openssl] Fix potential UAF in ossl_crypto_fixed_length_secure_compare
StringValue() can invoke an object's #to_str method, which may execute arbitrary Ruby code. If #to_str mutates the other string argument during comparison, its buffer may be reallocated, leaving a previously obtained RSTRING_PTR pointing to freed memory. This patch calls StringValue() on both arguments before retrieving their data pointers to prevent a potential use-after-free. ruby/openssl@c82c28c663
1 parent 8b96619 commit 998e2f4

File tree

2 files changed

+22
-4
lines changed

2 files changed

+22
-4
lines changed

ext/openssl/ossl.c

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -527,10 +527,18 @@ ossl_fips_mode_set(VALUE self, VALUE enabled)
527527
static VALUE
528528
ossl_crypto_fixed_length_secure_compare(VALUE dummy, VALUE str1, VALUE str2)
529529
{
530-
const unsigned char *p1 = (const unsigned char *)StringValuePtr(str1);
531-
const unsigned char *p2 = (const unsigned char *)StringValuePtr(str2);
532-
long len1 = RSTRING_LEN(str1);
533-
long len2 = RSTRING_LEN(str2);
530+
const unsigned char *p1;
531+
const unsigned char *p2;
532+
long len1;
533+
long len2;
534+
535+
StringValue(str1);
536+
StringValue(str2);
537+
538+
p1 = (const unsigned char *)RSTRING_PTR(str1);
539+
p2 = (const unsigned char *)RSTRING_PTR(str2);
540+
len1 = RSTRING_LEN(str1);
541+
len2 = RSTRING_LEN(str2);
534542

535543
if (len1 != len2) {
536544
ossl_raise(rb_eArgError, "inputs must be of equal length");

test/openssl/test_ossl.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,16 @@ def test_fixed_length_secure_compare
2424
assert_raise(ArgumentError) { OpenSSL.fixed_length_secure_compare("aaa", "bbbb") }
2525
end
2626

27+
def test_fixed_length_secure_compare_uaf
28+
str1 = "A" * 1000000
29+
evil_obj = Object.new
30+
evil_obj.define_singleton_method(:to_str) do
31+
str1.replace("C" * 1000000)
32+
"B" * 1000000
33+
end
34+
assert_false(OpenSSL.fixed_length_secure_compare(str1, evil_obj))
35+
end
36+
2737
def test_secure_compare
2838
assert_false(OpenSSL.secure_compare("aaa", "a"))
2939
assert_false(OpenSSL.secure_compare("aaa", "aa"))

0 commit comments

Comments
 (0)