[Bug dynamic-link/22723] New: SIGSEGV from unchecked NULL in DT_INIT_ARRAY during dlopen

["jreiser at BitWagon dot com" <sourceware-bugzilla at sourceware dot org>]

https://sourceware.org/bugzilla/show_bug.cgi?id=22723

            Bug ID: 22723
           Summary: SIGSEGV from unchecked NULL in DT_INIT_ARRAY during
                    dlopen
           Product: glibc
           Version: 2.28
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: dynamic-link
          Assignee: unassigned at sourceware dot org
          Reporter: jreiser at BitWagon dot com
  Target Milestone: ---

If the DT_INIT_ARRAY contains a NULL pointer then the call_init() routine of
_dl_init() (from dlopen()) will generate a SIGSEGV via:
===== glibc/elf/dl-init.c near line 70
      addrs = (ElfW(Addr) *) (init_array->d_un.d_ptr + l->l_addr);
      for (j = 0; j < jm; ++j)
        ((init_t) addrs[j]) (argc, argv, env);   /* SIGSEGV here */
=====

The simple and inexpensive guard:
        if (addrs[j]) {
          ((init_t) addrs[j]) (argc, argv, env);
        }
is sufficient to prevent such a SIGSEGV, and is consistent with the common
convention that NULL implies "no such object".  Other choices for response:
1) Give an error message and traceback on stderr, similar to "Duplicate free()"
or other errors in malloc+free, then abort().
2) Fail the dlopen(), such that dlerror() names the reason, the filename, and
the offset within the file of the offending pointer.

Of course the input file is malformed and its creator should fix it.  But
call_init should avoid generating SIGSEGV for this case.  There is a general
expectation of robustness which includes at least some validating of input. 
The possibility of malware or active security penetration, the existence of
__libc_enable_secure mode, and the possible embarrassment (or worse) of
admitting to a high-level manager or Board of Inquiry that "No, the code does
not check for NULL" all argue that it is insufficient to claim "the ABI says
that the pointers must be valid."

Here are two plausible scenarios that could cause a NULL in the DT_INIT_ARRAY:
1) The software toolchain allocated space for the pointer but forgot to
generate the Elf64_Rela relocation for it, or later dropped that relocation. 
[I have seen this case.]
2) The relocation designated a STB_WEAK symbol that was undefined at run time,
and thus the result value from relocation was 0 (NULL).  The possibility of
incomplete instantiation of C++ templates might contribute here.

For __libc_enable_secure mode, it can be argued that even more checking is
appropriate.  Is the pointer within a PT_LOAD segment that is executable (has
PF_X flag) and is within the current file being dlopen()ed?  Does the pointer
designate the entry point of some subroutine?  (Some architectures have a GATE
instruction or similar software conventions.)

Similar considerations apply to DT_PREINIT_ARRAY and DT_FINI_ARRAY, and
possibly DT_INIT itself.

-- 
You are receiving this mail because:
You are on the CC list for the bug.