-
Notifications
You must be signed in to change notification settings - Fork 190
Description
Commit b29e215 fixed a crash when d2i_TS_RESP_bio() fails by setting the DATA_PTR(self) pointer to NULL.
This only works if the failure of d2i_TS_RESP_bio() happens during the d2i call in OpenSSL because d2i takes ownership of the object and frees it on failure:
https://github.com/openssl/openssl/blob/3297773b040236baee9e68dd892fd8722b25afa1/crypto/asn1/tasn_dec.c#L145-L146
Prior to the d2i call, the OpenSSL code performs a read (note that d2i_TS_RESP_bio() is a wrapper of ASN1_d2i_bio()):
https://github.com/openssl/openssl/blob/3297773b040236baee9e68dd892fd8722b25afa1/crypto/asn1/a_d2i_fp.c#L45-L47
If the function fails at that point, then the d2i call wouldn't have gotten ownership of the object, and therefore the object will leak.
Example Valgrind trace:
72 (24 direct, 48 indirect) bytes in 1 blocks are definitely lost in loss record 17,824 of 25,777
malloc (at /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
CRYPTO_zalloc (at /usr/lib/x86_64-linux-gnu/libcrypto.so.3)
<unknown stack frame>
ASN1_item_new (at /usr/lib/x86_64-linux-gnu/libcrypto.so.3)
*ossl_ts_resp_alloc (ossl_ts.c:505)
<unknown stack frame>
rb_class_new_instance_pass_kw (at /usr/lib/x86_64-linux-gnu/libruby-3.2.so.3.2.3)
<unknown stack frame>
<unknown stack frame>
<unknown stack frame>
<unknown stack frame>
rb_vm_exec (at /usr/lib/x86_64-linux-gnu/libruby-3.2.so.3.2.3)
<unknown stack frame>
<unknown stack frame>
rb_catch_obj (at /usr/lib/x86_64-linux-gnu/libruby-3.2.so.3.2.3)
<unknown stack frame>
<unknown stack frame>
<unknown stack frame>
rb_vm_exec (at /usr/lib/x86_64-linux-gnu/libruby-3.2.so.3.2.3)
rb_yield (at /usr/lib/x86_64-linux-gnu/libruby-3.2.so.3.2.3)
rb_ary_each (at /usr/lib/x86_64-linux-gnu/libruby-3.2.so.3.2.3)
<unknown stack frame>
<unknown stack frame>
<unknown stack frame>
rb_vm_exec (at /usr/lib/x86_64-linux-gnu/libruby-3.2.so.3.2.3)
rb_yield (at /usr/lib/x86_64-linux-gnu/libruby-3.2.so.3.2.3)
rb_ary_each (at /usr/lib/x86_64-linux-gnu/libruby-3.2.so.3.2.3)
<unknown stack frame>
<unknown stack frame>
<unknown stack frame>
rb_vm_exec (at /usr/lib/x86_64-linux-gnu/libruby-3.2.so.3.2.3)
rb_yield (at /usr/lib/x86_64-linux-gnu/libruby-3.2.so.3.2.3)
rb_ary_each (at /usr/lib/x86_64-linux-gnu/libruby-3.2.so.3.2.3)
<unknown stack frame>
<unknown stack frame>
<unknown stack frame>
rb_vm_exec (at /usr/lib/x86_64-linux-gnu/libruby-3.2.so.3.2.3)
<unknown stack frame>
<unknown stack frame>
rb_catch_obj (at /usr/lib/x86_64-linux-gnu/libruby-3.2.so.3.2.3)
<unknown stack frame>
<unknown stack frame>
<unknown stack frame>
rb_vm_exec (at /usr/lib/x86_64-linux-gnu/libruby-3.2.so.3.2.3)
rb_vm_invoke_proc (at /usr/lib/x86_64-linux-gnu/libruby-3.2.so.3.2.3)
rb_proc_call_kw (at /usr/lib/x86_64-linux-gnu/libruby-3.2.so.3.2.3)
<unknown stack frame>
<unknown stack frame>
<unknown stack frame>
ruby_run_node (at /usr/lib/x86_64-linux-gnu/libruby-3.2.so.3.2.3)
This was found by a hybrid static-dynamic analyser that looks for inconsistent handling of error checks in bindings.