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.

Author: rhenium

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")

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 <openssl/opensslv.h>
 
 #include <openssl/err.h>

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;
 }

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)

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;

ext/openssl/ossl_engine.c

@@ -312,14 +312,19 @@ ossl_engine_load_privkey(int argc, VALUE *argv, VALUE self)
     ENGINE *e;
     EVP_PKEY *pkey;
     VALUE id, data, obj;
-    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);
     GetEngine(self, e);
-    pkey = ENGINE_load_private_key(e, sid, NULL, sdata);
-    if (!pkey) ossl_raise(eEngineError, NULL);
+    pkey = ENGINE_load_private_key(e,
+                                   NIL_P(id) ? NULL : rb_str_cstr(id),
+                                   NULL,
+                                   NIL_P(data) ? NULL : rb_str_cstr(data));
+    if (!pkey)
+        ossl_raise(eEngineError, "ENGINE_load_private_key");
     obj = ossl_pkey_wrap(pkey);
     OSSL_PKEY_SET_PRIVATE(obj);
 
@@ -341,14 +346,19 @@ ossl_engine_load_pubkey(int argc, VALUE *argv, VALUE self)
     ENGINE *e;
     EVP_PKEY *pkey;
     VALUE id, data;
-    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);
     GetEngine(self, e);
-    pkey = ENGINE_load_public_key(e, sid, NULL, sdata);
-    if (!pkey) ossl_raise(eEngineError, NULL);
+    pkey = ENGINE_load_public_key(e,
+                                  NIL_P(id) ? NULL : rb_str_cstr(id),
+                                  NULL,
+                                  NIL_P(data) ? NULL : rb_str_cstr(data));
+    if (!pkey)
+        ossl_raise(eEngineError, "ENGINE_load_public_key");
 
     return ossl_pkey_wrap(pkey);
 }
@@ -398,9 +408,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;
 }

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);

ext/openssl/ossl_pkcs12.c

@@ -90,16 +90,18 @@ 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;
     int nkey = 0, ncert = 0, kiter = 0, miter = 0, ktype = 0;
     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) {

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)

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;
 }