From 07a32ccea3201c5e5c9bbf94238e2b1a56f5c314 Mon Sep 17 00:00:00 2001 From: Kazuki Yamaguchi Date: Wed, 8 Jul 2026 00:01:46 +0900 Subject: [PATCH] Fix TOCTOU issues involving implicit type conversion A user can supply objects with malicious #to_str or #to_int methods to trigger a use-after-free. Reorder the input validation to prevent this. --- ext/openssl/extconf.rb | 2 ++ ext/openssl/ossl.h | 9 +++++++++ ext/openssl/ossl_asn1.c | 10 +++++----- ext/openssl/ossl_cipher.c | 4 +++- ext/openssl/ossl_config.c | 8 ++++---- ext/openssl/ossl_engine.c | 36 +++++++++++++++++++++++------------- ext/openssl/ossl_kdf.c | 6 +++--- ext/openssl/ossl_pkcs12.c | 16 +++++++++++----- ext/openssl/ossl_pkey.c | 10 ++++++---- ext/openssl/ossl_rand.c | 3 ++- ext/openssl/ossl_x509name.c | 16 +++++++++------- 11 files changed, 77 insertions(+), 43 deletions(-) diff --git a/ext/openssl/extconf.rb b/ext/openssl/extconf.rb index 6c7417899..b45feef2f 100644 --- a/ext/openssl/extconf.rb +++ b/ext/openssl/extconf.rb @@ -45,6 +45,8 @@ have_func("rb_io_maybe_wait(0, Qnil, Qnil, Qnil)", "ruby/io.h") # Ruby 3.2 have_func("rb_io_timeout", "ruby/io.h") +# Ruby 4.1 +have_func("rb_str_cstr(Qnil)", "ruby.h") Logging::message "=== Checking for system dependent stuff... ===\n" have_library("nsl", "t_open") diff --git a/ext/openssl/ossl.h b/ext/openssl/ossl.h index 452be5a2a..e2b125110 100644 --- a/ext/openssl/ossl.h +++ b/ext/openssl/ossl.h @@ -23,6 +23,15 @@ #define RUBY_TYPED_FROZEN_SHAREABLE 0 #endif +#ifndef HAVE_RB_STR_CSTR +static inline const char * +rb_str_cstr(VALUE str) +{ + RUBY_ASSERT(RB_TYPE_P(str, T_STRING)); + return StringValueCStr(str); +} +#endif + #include #include diff --git a/ext/openssl/ossl_asn1.c b/ext/openssl/ossl_asn1.c index 16eb6bd59..0f81f1ff0 100644 --- a/ext/openssl/ossl_asn1.c +++ b/ext/openssl/ossl_asn1.c @@ -1289,12 +1289,12 @@ ossl_asn1cons_each(VALUE self) static VALUE ossl_asn1obj_s_register(VALUE self, VALUE oid, VALUE sn, VALUE ln) { - StringValueCStr(oid); - StringValueCStr(sn); - StringValueCStr(ln); + StringValue(oid); + StringValue(sn); + StringValue(ln); - if(!OBJ_create(RSTRING_PTR(oid), RSTRING_PTR(sn), RSTRING_PTR(ln))) - ossl_raise(eASN1Error, NULL); + if (!OBJ_create(rb_str_cstr(oid), rb_str_cstr(sn), rb_str_cstr(ln))) + ossl_raise(eASN1Error, "OBJ_create"); return Qtrue; } diff --git a/ext/openssl/ossl_cipher.c b/ext/openssl/ossl_cipher.c index bed30ae3d..fb186590b 100644 --- a/ext/openssl/ossl_cipher.c +++ b/ext/openssl/ossl_cipher.c @@ -289,6 +289,7 @@ ossl_cipher_pkcs5_keyivgen(int argc, VALUE *argv, VALUE self) const EVP_MD *digest; VALUE vpass, vsalt, viter, vdigest, md_holder; unsigned char key[EVP_MAX_KEY_LENGTH], iv[EVP_MAX_IV_LENGTH], *salt = NULL; + unsigned char saltbuf[PKCS5_SALT_LEN]; int iter; rb_scan_args(argc, argv, "13", &vpass, &vsalt, &viter, &vdigest); @@ -297,7 +298,8 @@ ossl_cipher_pkcs5_keyivgen(int argc, VALUE *argv, VALUE self) StringValue(vsalt); if(RSTRING_LEN(vsalt) != PKCS5_SALT_LEN) ossl_raise(eCipherError, "salt must be an 8-octet string"); - salt = (unsigned char *)RSTRING_PTR(vsalt); + memcpy(saltbuf, RSTRING_PTR(vsalt), PKCS5_SALT_LEN); + salt = saltbuf; } iter = NIL_P(viter) ? 2048 : NUM2INT(viter); if (iter <= 0) diff --git a/ext/openssl/ossl_config.c b/ext/openssl/ossl_config.c index 274875a97..016bbf9cb 100644 --- a/ext/openssl/ossl_config.c +++ b/ext/openssl/ossl_config.c @@ -190,11 +190,11 @@ config_get_value(VALUE self, VALUE section, VALUE key) CONF *conf = GetConfig(self); const char *str, *sectionp; - StringValueCStr(section); - StringValueCStr(key); + StringValue(section); + StringValue(key); /* For compatibility; NULL means "default". */ - sectionp = RSTRING_LEN(section) ? RSTRING_PTR(section) : NULL; - str = NCONF_get_string(conf, sectionp, RSTRING_PTR(key)); + sectionp = RSTRING_LEN(section) ? rb_str_cstr(section) : NULL; + str = NCONF_get_string(conf, sectionp, rb_str_cstr(key)); if (!str) { ossl_clear_error(); return Qnil; diff --git a/ext/openssl/ossl_engine.c b/ext/openssl/ossl_engine.c index a2bcb07ea..8d7634a1d 100644 --- a/ext/openssl/ossl_engine.c +++ b/ext/openssl/ossl_engine.c @@ -312,14 +312,17 @@ ossl_engine_load_privkey(int argc, VALUE *argv, VALUE self) ENGINE *e; EVP_PKEY *pkey; VALUE id, data, obj; - char *sid, *sdata; + const char *sid, *sdata; rb_scan_args(argc, argv, "02", &id, &data); - sid = NIL_P(id) ? NULL : StringValueCStr(id); - sdata = NIL_P(data) ? NULL : StringValueCStr(data); + if (!NIL_P(id)) StringValue(id); + if (!NIL_P(data)) StringValue(data); + sid = NIL_P(id) ? NULL : rb_str_cstr(id); + sdata = NIL_P(data) ? NULL : rb_str_cstr(data); GetEngine(self, e); - pkey = ENGINE_load_private_key(e, sid, NULL, sdata); - if (!pkey) ossl_raise(eEngineError, NULL); + pkey = ENGINE_load_private_key(e, sid, NULL, (void *)sdata); + if (!pkey) + ossl_raise(eEngineError, "ENGINE_load_private_key"); obj = ossl_pkey_wrap(pkey); OSSL_PKEY_SET_PRIVATE(obj); @@ -341,14 +344,17 @@ ossl_engine_load_pubkey(int argc, VALUE *argv, VALUE self) ENGINE *e; EVP_PKEY *pkey; VALUE id, data; - char *sid, *sdata; + const char *sid, *sdata; rb_scan_args(argc, argv, "02", &id, &data); - sid = NIL_P(id) ? NULL : StringValueCStr(id); - sdata = NIL_P(data) ? NULL : StringValueCStr(data); + if (!NIL_P(id)) StringValue(id); + if (!NIL_P(data)) StringValue(data); + sid = NIL_P(id) ? NULL : rb_str_cstr(id); + sdata = NIL_P(data) ? NULL : rb_str_cstr(data); GetEngine(self, e); - pkey = ENGINE_load_public_key(e, sid, NULL, sdata); - if (!pkey) ossl_raise(eEngineError, NULL); + pkey = ENGINE_load_public_key(e, sid, NULL, (void *)sdata); + if (!pkey) + ossl_raise(eEngineError, "ENGINE_load_public_key"); return ossl_pkey_wrap(pkey); } @@ -398,9 +404,13 @@ ossl_engine_ctrl_cmd(int argc, VALUE *argv, VALUE self) GetEngine(self, e); rb_scan_args(argc, argv, "11", &cmd, &val); - ret = ENGINE_ctrl_cmd_string(e, StringValueCStr(cmd), - NIL_P(val) ? NULL : StringValueCStr(val), 0); - if (!ret) ossl_raise(eEngineError, NULL); + StringValue(cmd); + if (!NIL_P(val)) + StringValue(val); + ret = ENGINE_ctrl_cmd_string(e, rb_str_cstr(cmd), + NIL_P(val) ? NULL : rb_str_cstr(val), 0); + if (!ret) + ossl_raise(eEngineError, "ENGINE_ctrl_cmd_string"); return self; } diff --git a/ext/openssl/ossl_kdf.c b/ext/openssl/ossl_kdf.c index 99a3589b3..a8e03c06e 100644 --- a/ext/openssl/ossl_kdf.c +++ b/ext/openssl/ossl_kdf.c @@ -265,14 +265,14 @@ kdf_hkdf(int argc, VALUE *argv, VALUE self) rb_get_kwargs(opts, kwargs_ids, 4, 0, kwargs); StringValue(ikm); - ikmlen = RSTRING_LENINT(ikm); salt = StringValue(kwargs[0]); - saltlen = RSTRING_LENINT(salt); info = StringValue(kwargs[1]); - infolen = RSTRING_LENINT(info); len = (size_t)NUM2LONG(kwargs[2]); if (len > LONG_MAX) rb_raise(rb_eArgError, "length must be non-negative"); + ikmlen = RSTRING_LENINT(ikm); + saltlen = RSTRING_LENINT(salt); + infolen = RSTRING_LENINT(info); md = ossl_evp_md_fetch(kwargs[3], &md_holder); str = rb_str_new(NULL, (long)len); diff --git a/ext/openssl/ossl_pkcs12.c b/ext/openssl/ossl_pkcs12.c index da8fd1c3b..d3807873f 100644 --- a/ext/openssl/ossl_pkcs12.c +++ b/ext/openssl/ossl_pkcs12.c @@ -90,7 +90,7 @@ ossl_pkcs12_s_create(int argc, VALUE *argv, VALUE self) { VALUE pass, name, pkey, cert, ca, key_nid, cert_nid, key_iter, mac_iter, keytype; VALUE obj; - char *passphrase, *friendlyname; + const char *passphrase, *friendlyname; EVP_PKEY *key; X509 *x509; STACK_OF(X509) *x509s; @@ -98,8 +98,10 @@ ossl_pkcs12_s_create(int argc, VALUE *argv, VALUE self) PKCS12 *p12; rb_scan_args(argc, argv, "46", &pass, &name, &pkey, &cert, &ca, &key_nid, &cert_nid, &key_iter, &mac_iter, &keytype); - passphrase = NIL_P(pass) ? NULL : StringValueCStr(pass); - friendlyname = NIL_P(name) ? NULL : StringValueCStr(name); + if (!NIL_P(pass)) + StringValue(pass); + if (!NIL_P(name)) + StringValue(name); key = GetPKeyPtr(pkey); x509 = GetX509CertPtr(cert); /* TODO: make a VALUE to nid function */ @@ -117,6 +119,8 @@ ossl_pkcs12_s_create(int argc, VALUE *argv, VALUE self) miter = NUM2INT(mac_iter); if (!NIL_P(keytype)) ktype = NUM2INT(keytype); + passphrase = NIL_P(pass) ? NULL : rb_str_cstr(pass); + friendlyname = NIL_P(name) ? NULL : rb_str_cstr(name); #if defined(OPENSSL_IS_AWSLC) if (ktype != 0) { @@ -177,7 +181,7 @@ ossl_pkcs12_initialize(int argc, VALUE *argv, VALUE self) PKCS12 *p12; BIO *in; VALUE arg, pass, pkey, cert, ca; - char *passphrase; + const char *passphrase; EVP_PKEY *key; X509 *x509; STACK_OF(X509) *x509s = NULL; @@ -192,7 +196,8 @@ ossl_pkcs12_initialize(int argc, VALUE *argv, VALUE self) RTYPEDDATA_DATA(self) = p12; return self; } - passphrase = NIL_P(pass) ? NULL : StringValueCStr(pass); + if (!NIL_P(pass)) + StringValue(pass); in = ossl_obj2bio(&arg); p12 = d2i_PKCS12_bio(in, NULL); BIO_free(in); @@ -201,6 +206,7 @@ ossl_pkcs12_initialize(int argc, VALUE *argv, VALUE self) RTYPEDDATA_DATA(self) = p12; pkey = cert = ca = Qnil; + passphrase = NIL_P(pass) ? NULL : rb_str_cstr(pass); if (!PKCS12_parse(p12, passphrase, &key, &x509, &x509s)) ossl_raise(ePKCS12Error, "PKCS12_parse"); if (key) { diff --git a/ext/openssl/ossl_pkey.c b/ext/openssl/ossl_pkey.c index 80b207317..dd83639c5 100644 --- a/ext/openssl/ossl_pkey.c +++ b/ext/openssl/ossl_pkey.c @@ -251,9 +251,10 @@ pkey_ctx_apply_options_i(RB_BLOCK_CALL_FUNC_ARGLIST(i, ctx_v)) if (SYMBOL_P(key)) key = rb_sym2str(key); + StringValue(key); value = rb_String(value); - if (EVP_PKEY_CTX_ctrl_str(ctx, StringValueCStr(key), StringValueCStr(value)) <= 0) + if (EVP_PKEY_CTX_ctrl_str(ctx, rb_str_cstr(key), rb_str_cstr(value)) <= 0) ossl_raise(ePKeyError, "EVP_PKEY_CTX_ctrl_str(ctx, %+"PRIsVALUE", %+"PRIsVALUE")", key, value); return Qnil; @@ -641,7 +642,6 @@ lookup_pkey_type(VALUE type) const EVP_PKEY_ASN1_METHOD *ameth; int pkey_id; - StringValue(type); /* * XXX: EVP_PKEY_asn1_find_str() looks up a PEM type string. Should we use * OBJ_txt2nid() instead (and then somehow check if the NID is an acceptable @@ -670,11 +670,12 @@ ossl_pkey_new_raw_private_key(VALUE self, VALUE type, VALUE key) EVP_PKEY *pkey; size_t keylen; + StringValue(type); StringValue(key); keylen = RSTRING_LEN(key); #ifdef OSSL_USE_PROVIDER - pkey = EVP_PKEY_new_raw_private_key_ex(NULL, StringValueCStr(type), NULL, + pkey = EVP_PKEY_new_raw_private_key_ex(NULL, rb_str_cstr(type), NULL, (unsigned char *)RSTRING_PTR(key), keylen); if (!pkey) @@ -702,11 +703,12 @@ ossl_pkey_new_raw_public_key(VALUE self, VALUE type, VALUE key) EVP_PKEY *pkey; size_t keylen; + StringValue(type); StringValue(key); keylen = RSTRING_LEN(key); #ifdef OSSL_USE_PROVIDER - pkey = EVP_PKEY_new_raw_public_key_ex(NULL, StringValueCStr(type), NULL, + pkey = EVP_PKEY_new_raw_public_key_ex(NULL, rb_str_cstr(type), NULL, (unsigned char *)RSTRING_PTR(key), keylen); if (!pkey) diff --git a/ext/openssl/ossl_rand.c b/ext/openssl/ossl_rand.c index 753f8b25f..c24b87975 100644 --- a/ext/openssl/ossl_rand.c +++ b/ext/openssl/ossl_rand.c @@ -52,8 +52,9 @@ ossl_rand_seed(VALUE self, VALUE str) static VALUE ossl_rand_add(VALUE self, VALUE str, VALUE entropy) { + double randomness = NUM2DBL(entropy); StringValue(str); - RAND_add(RSTRING_PTR(str), RSTRING_LENINT(str), NUM2DBL(entropy)); + RAND_add(RSTRING_PTR(str), RSTRING_LENINT(str), randomness); return self; } diff --git a/ext/openssl/ossl_x509name.c b/ext/openssl/ossl_x509name.c index 23363b695..803b7c5bd 100644 --- a/ext/openssl/ossl_x509name.c +++ b/ext/openssl/ossl_x509name.c @@ -195,26 +195,28 @@ static VALUE ossl_x509name_add_entry(int argc, VALUE *argv, VALUE self) { X509_NAME *name; - VALUE oid, value, type, opts, kwargs[2]; + VALUE oid, value, vtype, opts, kwargs[2]; static ID kwargs_ids[2]; - const char *oid_name; - int loc = -1, set = 0; + int type, loc = -1, set = 0; if (!kwargs_ids[0]) { kwargs_ids[0] = rb_intern_const("loc"); kwargs_ids[1] = rb_intern_const("set"); } - rb_scan_args(argc, argv, "21:", &oid, &value, &type, &opts); + rb_scan_args(argc, argv, "21:", &oid, &value, &vtype, &opts); rb_get_kwargs(opts, kwargs_ids, 0, 2, kwargs); - oid_name = StringValueCStr(oid); + StringValue(oid); StringValue(value); - if(NIL_P(type)) type = rb_aref(OBJECT_TYPE_TEMPLATE, oid); + if (!NIL_P(vtype)) + type = NUM2INT(vtype); + else + type = NUM2INT(rb_aref(OBJECT_TYPE_TEMPLATE, oid)); if (kwargs[0] != Qundef) loc = NUM2INT(kwargs[0]); if (kwargs[1] != Qundef) set = NUM2INT(kwargs[1]); GetX509Name(self, name); - if (!X509_NAME_add_entry_by_txt(name, oid_name, NUM2INT(type), + if (!X509_NAME_add_entry_by_txt(name, rb_str_cstr(oid), type, (unsigned char *)RSTRING_PTR(value), RSTRING_LENINT(value), loc, set)) ossl_raise(eX509NameError, "X509_NAME_add_entry_by_txt");