From 452181fe7420410a64b3e6e390caa5a9f96bcff4 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Aleksey=20Kliger=20=28=CE=BBgeek=29?= Date: Fri, 19 Apr 2019 15:57:31 -0400 Subject: [PATCH] [loader] Don't store through a null ptr (mono/mono#14110) * [loader] Dont't store through a null ptr If 'status' is null, don't try to write to it. Fixes https://github.com/mono/mono/issues/13941 * [metadata] Protect writes to MonoImageOpenStatus* in a few places. When possible assert that `MonoImageOpenStatus *status` arguments to various functions in the runtime aren't null. There are a few places (MONO_API) where it's possible that callers are passing null. In that case, use a local MonoImageOpenStatus, or check for null before doing assignments. Commit migrated from https://github.com/mono/mono/commit/7ba1f77f6da6375bd941b6bac0bbab3bb30ddd6b --- src/mono/mono/metadata/assembly.c | 24 +++++++++++++++++++++++- src/mono/mono/metadata/image.c | 10 ++++++---- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/mono/mono/metadata/assembly.c b/src/mono/mono/metadata/assembly.c index abd9c19..46b474d 100644 --- a/src/mono/mono/metadata/assembly.c +++ b/src/mono/mono/metadata/assembly.c @@ -1464,6 +1464,7 @@ load_reference_by_aname_refonly_asmctx (MonoAssemblyName *aname, MonoAssembly *a { MonoAssembly *reference = NULL; g_assert (assm != NULL); + g_assert (status != NULL); *status = MONO_IMAGE_OK; { /* We use the loaded corlib */ @@ -1494,6 +1495,7 @@ static MonoAssembly* load_reference_by_aname_default_asmctx (MonoAssemblyName *aname, MonoAssembly *assm, MonoImageOpenStatus *status) { MonoAssembly *reference = NULL; + g_assert (status != NULL); *status = MONO_IMAGE_OK; { /* we first try without setting the basedir: this can eventually result in a ResolveAssembly @@ -1539,6 +1541,8 @@ load_reference_by_aname_individual_asmctx (MonoAssemblyName *aname, MonoAssembly * subject to remaping and binding. */ + g_assert (status != NULL); + MonoAssembly *reference = NULL; *status = MONO_IMAGE_OK; MonoAssemblyName maped_aname; @@ -1739,7 +1743,8 @@ void mono_assembly_load_references (MonoImage *image, MonoImageOpenStatus *status) { /* This is a no-op now but it is part of the embedding API so we can't remove it */ - *status = MONO_IMAGE_OK; + if (status) + *status = MONO_IMAGE_OK; } typedef struct AssemblyLoadHook AssemblyLoadHook; @@ -2591,6 +2596,8 @@ chain_redirections_loadfrom (MonoImage *image, MonoImageOpenStatus *out_status) MonoAssembly* mono_assembly_binding_applies_to_image (MonoImage* image, MonoImageOpenStatus *status) { + g_assert (status != NULL); + /* This is a "fun" one now. * For LoadFrom ("/basedir/some.dll") or LoadFile("/basedir/some.dll") or Load(byte[])), * apparently what we're meant to do is: @@ -2658,6 +2665,8 @@ mono_assembly_binding_applies_to_image (MonoImage* image, MonoImageOpenStatus *s MonoAssembly* mono_problematic_image_reprobe (MonoImage *image, MonoImageOpenStatus *status) { + g_assert (status != NULL); + if (G_LIKELY (!mono_is_problematic_image (image))) { *status = MONO_IMAGE_OK; return NULL; @@ -2753,6 +2762,9 @@ mono_assembly_load_from_full (MonoImage *image, const char*fname, MonoAssembly *res; MONO_ENTER_GC_UNSAFE; MonoAssemblyLoadRequest req; + MonoImageOpenStatus def_status; + if (!status) + status = &def_status; mono_assembly_request_prepare (&req, sizeof (req), refonly ? MONO_ASMCTX_REFONLY : MONO_ASMCTX_DEFAULT); res = mono_assembly_request_load_from (image, fname, &req, status); MONO_EXIT_GC_UNSAFE; @@ -2771,6 +2783,8 @@ mono_assembly_request_load_from (MonoImage *image, const char *fname, MonoAssembly *ass, *ass2; char *base_dir; + g_assert (status != NULL); + asmctx = req->asmctx; predicate = req->predicate; user_data = req->predicate_ud; @@ -2941,6 +2955,9 @@ mono_assembly_load_from (MonoImage *image, const char *fname, MonoAssembly *res; MONO_ENTER_GC_UNSAFE; MonoAssemblyLoadRequest req; + MonoImageOpenStatus def_status; + if (!status) + status = &def_status; mono_assembly_request_prepare (&req, sizeof (req), MONO_ASMCTX_DEFAULT); res = mono_assembly_request_load_from (image, fname, &req, status); MONO_EXIT_GC_UNSAFE; @@ -3595,6 +3612,9 @@ mono_assembly_load_with_partial_name (const char *name, MonoImageOpenStatus *sta { MonoAssembly *result; MONO_ENTER_GC_UNSAFE; + MonoImageOpenStatus def_status; + if (!status) + status = &def_status; result = mono_assembly_load_with_partial_name_internal (name, status); MONO_EXIT_GC_UNSAFE; return result; @@ -3612,6 +3632,8 @@ mono_assembly_load_with_partial_name_internal (const char *name, MonoImageOpenSt MONO_REQ_GC_UNSAFE_MODE; + g_assert (status != NULL); + memset (&base_name, 0, sizeof (MonoAssemblyName)); aname = &base_name; diff --git a/src/mono/mono/metadata/image.c b/src/mono/mono/metadata/image.c index b77cb34..6d5782f 100644 --- a/src/mono/mono/metadata/image.c +++ b/src/mono/mono/metadata/image.c @@ -1386,7 +1386,8 @@ do_mono_image_load (MonoImage *image, MonoImageOpenStatus *status, mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_ASSEMBLY, "Loading problematic image %s", image->name); } else { mono_trace (G_LOG_LEVEL_INFO, MONO_TRACE_ASSEMBLY, "Denying load of problematic image %s", image->name); - *status = MONO_IMAGE_IMAGE_INVALID; + if (status) + *status = MONO_IMAGE_IMAGE_INVALID; goto invalid_image; } } @@ -1790,9 +1791,10 @@ mono_image_open_a_lot (const char *fname, MonoImageOpenStatus *status, gboolean g_assert (!image); g_free (absfname); if (status) { - if (last_error == ERROR_BAD_EXE_FORMAT || last_error == STATUS_INVALID_IMAGE_FORMAT) - *status = MONO_IMAGE_IMAGE_INVALID; - else { + if (last_error == ERROR_BAD_EXE_FORMAT || last_error == STATUS_INVALID_IMAGE_FORMAT) { + if (status) + *status = MONO_IMAGE_IMAGE_INVALID; + } else { if (last_error == ERROR_FILE_NOT_FOUND || last_error == ERROR_PATH_NOT_FOUND) mono_set_errno (ENOENT); else -- 2.7.4