How to solve process control reported by Fortify

476 Views Asked by At

I'm working on below process control issue reported by fortify which is described here.

The function load() in filename.c calls dlopen() on line 3. The call loads a library without specifying an absolute path. It could result in the program using a malicious library supplied by an attacker.

I have below function which is getting invoked in different places of code.

void* load(char* name)
{
    void* handle;
    handle = dlopen(name, RTLD_LAZY | RTLD_GLOBAL);
    return(handle);
}

void somefunc()
{
    void *login_module_handle = load("/home/myuser/load_this_shared_lib.so");
}

Here I'm already using absolute path, but don't understand why Fortify still reports the error.

The possible recommendation from Fortify is as shown below.

  1. Whenever possible, libraries should be controlled by the application and executed using an absolute path.
  2. In cases where the path is not known at compile time, such as for cross-platform applications, an absolute path should be constructed from known values during execution.

Any suggestion would be helpful.

2

There are 2 best solutions below

0
On

There are two related but separate problems here:

  1. How to load dynamic libraries in a secure manner at run time

  2. How to make Fortify see security is properly taken care of


The common approach that Fortify recommends, is to define a fixed path to a directory, in which plugins must be located. It is assumed that only those with sufficient privileges have access to that directory. The pattern is similar to

#define _GNU_SOURCE

#ifndef  PLUGIN_PATH
#define  PLUGIN_PATH  "/usr/lib/thisapp/plugins"
#endif
#ifndef  PLUGIN_SUFFIX
#define  PLUGIN_SUFFIX  ".so"
#endif


// Try to load dynamic library 'name' under PLUGIN_PATH.
// Returns NULL if failed, with errno set.
// If this returns NULL and errno==0, use dlerror() to
// get the error message.
void *load(const char *name)
{
    // Empty or NULL name is not allowed, and it may not start with '.'.
    if (!name || !*name || *name == '.') {
        errno = EINVAL;
        return NULL;
    }

    // Name may not contain '/'.
    if (strchr(name, '/')) {
        errno = EINVAL;
        return NULL;
    }

    char *path = NULL;
    int pathlen = asprintf(&path, "%s/%s%s", PLUGIN_PATH, name, PLUGIN_SUFFIX);
    if (pathlen < 1 || !path) {
        errno = ENOMEM;
        return NULL;
    }

    void *handle = dlopen(path, RTLD_LAZY | RTLD_GLOBAL);
    free(path);
    errno = 0;  /* If handle is NULL, look at dlerror() */
    return handle;
}

In general, a fixed path named after the application in the /usr/lib tree, say /usr/lib/application/plugins/ can be considered secure, assuming it is installed so the directory and all its parent directories are owned by superuser and a privileged group, with no write access to the others (i.e., drwxrwxr-x or drwr-xr-x).

If the user can specify full path to the library, each directory along the path must be vetted, to verify if they may be modified by unprivileged users. If any are suspect, then the library is not secure.

The library file itself can be opened read-only, and a read lease (see fcntl()) placed on it, if the process has sufficient privileges (CAP_LEASE capability, or run as a privileged user). This will fail if any process has the file open for writing, meaning another process is able to modify it while we use it. If the lease is granted, then any other process trying to open the file for writing causes them to block, and this process be notified by a signal. This process can delay (but not fully block) the other process from opening the library, for up to lease-break-time, typically 45 seconds. This way, a secure process can detect if someone (even privileged) tries to modify the plugin while it is being used.

With a valid read lease, it is time to examine the file ownership and mode (via fstat() using the file descriptor used to open it read-only), to see if it is owned and only modifiable by privileged users. If it is modifiable by unprivileged users, it is not secure.

After all the above checks, given the file descriptor number FD, the path provided to dlopen() is /proc/self/fd/FD. This reuses the same file descriptor, ensuring we do not have a TOCTTOU race window (time of check to time of use).


Whether Fortify recognizes the above measures, or just complains whenever it sees dlopen() used at all, I have no idea: I do not use it myself.

2
On

The load function could in theory be called someplace else that doesn't pass a full path or, even worse, a variable populated by the user that isn't properly checked.

load might better be implemented as a macro in this case:

#define load(name) dlopen(name, RTLD_LAZY | RTLD_GLOBAL)

Then when the substitution happens, dlopen is actually given a string constant specifying an absolute path that Fortify should be able to see.