Skip to content

Conversation

@Lekensteyn
Copy link

Do not assume an existing /tmp/kheaders-$(uname -r) directory to contain headers. By default, systemd removes files older than 10 days via tmpfiles config. If a header file is missing, proceed by removing the old directory before attempting a new extraction.

This also fixes a vulnerability where bcc could be convinced to use a kheaders directory of an unprivileged user:

  1. Attacker creates a kheaders dir symlink to a non-existing file. This bypasses file_exists and non-root user checks in get_proc_kheaders.

  2. extract_kheaders proceeds to extract a headers tarball to a temporary directory. Within this race window, an attacker can remove the symlink, and replace it by a directory under its control.

  3. extract_kheaders returns success even though the directory is under control of an attacker:

    // fails because the attacker created the directory.
    ret = rename(dirpath_tmp, dirpath.c_str());
    if (ret)
      // OOPS: return code is overwritten when tmp dir is removed
      ret = system(("rm -rf " + std::string(dirpath_tmp)).c_str());
    

Fix by ignoring temporary directory removal failures, and by checking whether a parallel process created a legitimate privileged directory with the expected header files.

All privileged directory checks do not follow symlinks. Remove the owner check for PROC_KHEADERS_PATH as we assume /sys to be trusted.

Fixes #4213

Fixes: 008ea09 ("clang: check header ownership (#4928)")

Do not assume an existing `/tmp/kheaders-$(uname -r)` directory to
contain headers. By default, systemd removes files older than 10 days
via tmpfiles config. If a header file is missing, proceed by removing
the old directory before attempting a new extraction.

This also fixes a vulnerability where bcc could be convinced to use a
kheaders directory of an unprivileged user:

 1. Attacker creates a kheaders dir symlink to a non-existing file. This
    bypasses file_exists and non-root user checks in get_proc_kheaders.
 2. extract_kheaders proceeds to extract a headers tarball to a
    temporary directory. Within this race window, an attacker can remove
    the symlink, and replace it by a directory under its control.
 3. extract_kheaders returns success even though the directory is under
    control of an attacker:

        // fails because the attacker created the directory.
        ret = rename(dirpath_tmp, dirpath.c_str());
        if (ret)
          // OOPS: return code is overwritten when tmp dir is removed
          ret = system(("rm -rf " + std::string(dirpath_tmp)).c_str());

Fix by ignoring temporary directory removal failures, and by checking
whether a parallel process created a legitimate privileged directory
with the expected header files.

All privileged directory checks do not follow symlinks. Remove the
owner check for PROC_KHEADERS_PATH as we assume /sys to be trusted.

Fixes iovisor#4213

Fixes: 008ea09 ("clang: check header ownership (iovisor#4928)")
Signed-off-by: Peter Wu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent /tmp/kheaders directory makes bcc fail

1 participant