Page MenuHomePhabricator

enlightenment_system: setcwd() is missing and a check for "libcurl.so.5" allows arbitrary file existence tests
Closed, ResolvedPublic

Description

g) setcwd() is missing and a check for "libcurl.so.5" allows arbitrary file
existence tests

The setuid-root program does not reset the current working directory to a safe
path like "/". Therefore whichever directory the unprivileged user sits in
when invoking the setuid-root program will affect relative path lookups
performed in the setuid-root context.

One such instance is found in the context of the call to
`ecore_file_download_init()´ which later down the path tries to load the
shared library "libcurl.so.5" via eina_module_load(). If the dlopen()
fails the latter function performs a stat() on the library basename "to
determine" whether the library existed or not. If it thinks the library
existed but couldn't be loaded then it prints out an error message.

An attacker can place a symlink named "libcurl.so.5" in the current working
directory and enlightenment_system will follow the link and behave differently
depending on whether the link target exists or not. Therefore an attacker can
test for existence of arbitrary files also in paths usually not accessible to
him. Example:

user /home/user $ /usr/lib64/enlightenment/utils/enlightenment_system 
ddc-list^C

user /home/user $ ln -s /root/.bashrc libcurl.so.5
user /home/user $ /usr/lib64/enlightenment/utils/enlightenment_system
ERR<4165>:eina_module ../src/lib/eina/eina_module.c:319 eina_module_load()
could not dlopen("libcurl.so.5", libcurl.so.5: cannot open shared object file:
No such file or directory): RTLD_NOW
[...]

I suggest to set the CWD to a defined and safe default like "/".

Furthermore I'd adjust the code in eina_module_load() to use dlerror()
instead of guessing reasons for failed library loads.

http://bugzilla.suse.com/show_bug.cgi?id=1170174

simotek created this task.Apr 22 2020, 4:06 AM
simotek triaged this task as High priority.

btw eina module does use dlerror:

if (!stat(m->file, &st))
  ERR("could not dlopen(\"%s\", %s): %s", m->file, dlerror(),
      (flag == RTLD_NOW) ? "RTLD_NOW" : "RTLD_LAZY");
else
  DBG("could not dlopen(\"%s\", %s): %s", m->file, dlerror(),
      (flag == RTLD_NOW) ? "RTLD_NOW" : "RTLD_LAZY");

it just upgrades it to an error vs debug message if the file exists ... a non-existing file is just fine - we can just dumbly load the module and then check return for its existence.

btw eina module does use dlerror:

True. The problem seems to be that dlerror() is not allowing programmatical error handling. stat()ing the path for additional diagnostic is also no ideal solution, however.