-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Description
During the development of #111645 I found various type confusion issues in our existing Mono source code. Backporting the fixes for these issues is not necessarily worth it since the resulting behavior change could expose new bugs or change application behavior, but it's worth tracking all of the latent issues here in case we need to fix them later to address customer-facing issues.
There are probably more errors of this type that I didn't find.
runtime/src/mono/mono/metadata/metadata.c
Line 7028 in 109c946
| if ((m_class_get_element_class (type->data.klass) == mono_defaults.char_class) && !unicode) |
type is either ARRAY or SZARRAY; the two types have different internal representations so the access to klass here is incorrect.
runtime/src/mono/mono/metadata/marshal.c
Line 6447 in 109c946
| mono_unichar2 *utf16_array = g_utf8_to_utf16 ((const char *)ptr, arr->max_length, NULL, NULL, NULL); |
utf16_array can be NULL and that scenario is not handled. The current implementation could potentially erroneously mess with the heap in the zero page on WASM; on other targets it should fail-fast.
runtime/src/mono/mono/mini/mini.c
Line 1068 in 109c946
| /* Fall through */ |
runtime/src/mono/mono/mini/mini.c
Line 1379 in 109c946
| /* Fall through */ |
runtime/src/mono/mono/mini/mini-amd64.c
Line 312 in 109c946
| /* fall through */ |
The fallthrough from
GENERICINST to VALUETYPE results in various accesses to klass being incorrect. Note that there are many functions with the same error.
| if (m_class_is_valuetype (t->data.klass)) { |
The use of
klass on a GENERICINST is incorrect.
runtime/src/mono/mono/mini/mini-generic-sharing.c
Line 3621 in 109c946
| (sig->ret->type == MONO_TYPE_CLASS && !strcmp (m_class_get_name (sig->ret->data.generic_class->container_class), "Task")) || |
This copy-paste error treats as
CLASS as a GENERICINST and as a result the comparison against Task will probably never succeed and could crash instead.
runtime/src/mono/mono/metadata/icall.c
Line 6596 in 109c946
| MonoType blob_type; |
runtime/src/mono/mono/metadata/reflection.c
Line 1033 in 109c946
| MonoType blob_type; |
These stack-allocated
MonoTypes are not fully initialized which means the additional data in the type is random garbage from the stack.
runtime/src/mono/mono/metadata/reflection.c
Line 1045 in 109c946
| blob_type.data.klass = mono_class_from_mono_type_internal (&blob_type); |
This call to
mono_class_from_mono_type_internal is being passed &blob_type which does not match the pattern from icall.c or from its neighboring calls, so it's possibly (but not certainly) an error.
| case MONO_TYPE_ARRAY: |
Fallthrough for
ARRAY and SZARRAY is incorrect since they have different internal representations.