From 3b51e7933ba42b3299ba0678ce1b4d8a844064cd Mon Sep 17 00:00:00 2001 From: Doug Hogan Date: Mon, 14 Jan 2019 21:44:49 -0800 Subject: enif_release_resource is not NULL safe * Add if checks and update coccinelle script. --- lib/crypto/c_src/aes.c | 6 ++++-- lib/crypto/c_src/chacha20.c | 6 ++++-- lib/crypto/c_src/check_erlang.cocci | 28 +++++++++++++++++++++++++--- lib/crypto/c_src/engine.c | 9 ++++++--- lib/crypto/c_src/hash.c | 6 ++++-- lib/crypto/c_src/hmac.c | 3 ++- 6 files changed, 45 insertions(+), 13 deletions(-) diff --git a/lib/crypto/c_src/aes.c b/lib/crypto/c_src/aes.c index 94b874298e..205cb8b058 100644 --- a/lib/crypto/c_src/aes.c +++ b/lib/crypto/c_src/aes.c @@ -223,7 +223,8 @@ ERL_NIF_TERM aes_ctr_stream_init(ErlNifEnv* env, int argc, const ERL_NIF_TERM ar ret = enif_make_badarg(env); done: - enif_release_resource(ctx); + if (ctx) + enif_release_resource(ctx); return ret; } @@ -270,7 +271,8 @@ ERL_NIF_TERM aes_ctr_stream_encrypt(ErlNifEnv* env, int argc, const ERL_NIF_TERM ret = enif_make_badarg(env); done: - enif_release_resource(new_ctx); + if (new_ctx) + enif_release_resource(new_ctx); return ret; } diff --git a/lib/crypto/c_src/chacha20.c b/lib/crypto/c_src/chacha20.c index 03b8589524..51a24a43e9 100644 --- a/lib/crypto/c_src/chacha20.c +++ b/lib/crypto/c_src/chacha20.c @@ -63,7 +63,8 @@ ERL_NIF_TERM chacha20_stream_init(ErlNifEnv* env, int argc, const ERL_NIF_TERM a ret = enif_make_badarg(env); done: - enif_release_resource(ctx); + if (ctx) + enif_release_resource(ctx); return ret; #else @@ -113,7 +114,8 @@ ERL_NIF_TERM chacha20_stream_crypt(ErlNifEnv* env, int argc, const ERL_NIF_TERM ret = enif_make_badarg(env); done: - enif_release_resource(new_ctx); + if (new_ctx) + enif_release_resource(new_ctx); return ret; #else diff --git a/lib/crypto/c_src/check_erlang.cocci b/lib/crypto/c_src/check_erlang.cocci index bdb99d5bb0..eba0e3935c 100644 --- a/lib/crypto/c_src/check_erlang.cocci +++ b/lib/crypto/c_src/check_erlang.cocci @@ -39,7 +39,8 @@ position p; goto L; ... when strict, forall - enif_release_resource(CTX) + if (CTX) + enif_release_resource(CTX); // After calling enif_alloc_binary(), you must either release it with @@ -93,7 +94,7 @@ position pa, pm, pr; // enif_has_pending_exception returns true if exception pending @erlang_check_void@ -identifier FUNCVOID =~ "^(enif_free|enif_mutex_destroy|enif_mutex_lock|enif_mutex_unlock|enif_release_binary|enif_release_resource|enif_rwlock_destroy|enif_rwlock_rlock|enif_rwlock_runlock|enif_rwlock_rwlock|enif_rwlock_rwunlock|enif_system_info)$"; +identifier FUNCVOID =~ "^(enif_free|enif_mutex_destroy|enif_mutex_lock|enif_mutex_unlock|enif_release_binary|enif_rwlock_destroy|enif_rwlock_rlock|enif_rwlock_runlock|enif_rwlock_rwlock|enif_rwlock_rwunlock|enif_system_info)$"; position p; @@ @@ -133,6 +134,22 @@ position p; ) +@erlang_check_null_free@ +expression X; +identifier FUNCFREE =~ "^(enif_release_resource)$"; +position p; +@@ + + if ( +( + X +| + X != NULL +) + ) + FUNCFREE(X)@p; + + @erlang_check_new@ expression RET; identifier FUNCNEW =~ "^(enif_make_atom|enif_make_badarg|enif_make_binary|enif_make_int|enif_make_list|enif_make_list_from_array|enif_make_resource|enif_make_tuple|enif_raise_exception|enif_schedule_nif|enif_thread_self)$"; @@ -149,7 +166,7 @@ position p; // Flag any calls that aren't part of the above pattern. @enif_alloc_not_free@ -identifier FUNCVOID =~ "^(enif_free|enif_mutex_destroy|enif_mutex_lock|enif_mutex_unlock|enif_release_binary|enif_release_resource|enif_rwlock_destroy|enif_rwlock_rlock|enif_rwlock_runlock|enif_rwlock_rwlock|enif_rwlock_rwunlock|enif_system_info)$"; +identifier FUNCVOID =~ "^(enif_free|enif_mutex_destroy|enif_mutex_lock|enif_mutex_unlock|enif_release_binary|enif_rwlock_destroy|enif_rwlock_rlock|enif_rwlock_runlock|enif_rwlock_rwlock|enif_rwlock_rwunlock|enif_system_info)$"; position pvoid != {erlang_check_void.p,enif_alloc_binary.pr}; identifier FUNCNULL =~ "^(enif_alloc|enif_alloc_resource|enif_dlopen|enif_dlsym|enif_make_new_binary|enif_mutex_create|enif_open_resource_type|enif_realloc|enif_rwlock_create)$"; @@ -161,6 +178,9 @@ position pnot != {erlang_check_not.p,enif_alloc_binary.pa}; identifier FUNCNEW =~ "^(enif_make_atom|enif_make_badarg|enif_make_binary|enif_make_int|enif_make_list|enif_make_list_from_array|enif_make_resource|enif_make_tuple|enif_raise_exception|enif_schedule_nif|enif_thread_self)$"; position pnew != {erlang_check_new.p,enif_alloc_binary.pm}; +identifier FUNCFREE =~ "^(enif_release_resource)$"; +position pfree != {enif_alloc_resource.p,erlang_check_null_free.p}; + @@ ( @@ -171,4 +191,6 @@ position pnew != {erlang_check_new.p,enif_alloc_binary.pm}; * FUNCNOT(...)@pnot | * FUNCNEW(...)@pnew +| +* FUNCFREE(...)@pfree ) diff --git a/lib/crypto/c_src/engine.c b/lib/crypto/c_src/engine.c index 44648280b4..90fc8b66cd 100644 --- a/lib/crypto/c_src/engine.c +++ b/lib/crypto/c_src/engine.c @@ -158,7 +158,8 @@ ERL_NIF_TERM engine_by_id_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[ done: if (engine_id) enif_free(engine_id); - enif_release_resource(ctx); + if (ctx) + enif_release_resource(ctx); return ret; #else @@ -604,7 +605,8 @@ ERL_NIF_TERM engine_get_first_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM a ret = enif_make_badarg(env); done: - enif_release_resource(ctx); + if (ctx) + enif_release_resource(ctx); return ret; #else @@ -647,7 +649,8 @@ ERL_NIF_TERM engine_get_next_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM ar ret = enif_make_badarg(env); done: - enif_release_resource(next_ctx); + if (next_ctx) + enif_release_resource(next_ctx); return ret; #else diff --git a/lib/crypto/c_src/hash.c b/lib/crypto/c_src/hash.c index a90957f1a7..000eae047a 100644 --- a/lib/crypto/c_src/hash.c +++ b/lib/crypto/c_src/hash.c @@ -130,7 +130,8 @@ ERL_NIF_TERM hash_init_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]) ret = atom_notsup; done: - enif_release_resource(ctx); + if (ctx) + enif_release_resource(ctx); return ret; } @@ -167,7 +168,8 @@ ERL_NIF_TERM hash_update_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[] ret = atom_notsup; done: - enif_release_resource(new_ctx); + if (new_ctx) + enif_release_resource(new_ctx); return ret; } diff --git a/lib/crypto/c_src/hmac.c b/lib/crypto/c_src/hmac.c index a5ff35c706..9418e7950e 100644 --- a/lib/crypto/c_src/hmac.c +++ b/lib/crypto/c_src/hmac.c @@ -169,7 +169,8 @@ ERL_NIF_TERM hmac_init_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]) ret = atom_notsup; done: - enif_release_resource(obj); + if (obj) + enif_release_resource(obj); return ret; } -- cgit v1.2.3