Skip to content

Commit

Permalink
configuration: fix handling relative executables
Browse files Browse the repository at this point in the history
Commit 720eb98 (configuration: refactorize code scanning for
executables) introduced a bug when the executable is a relative path.
In this case the file is always expanded, thus causing file to be always
considered non executable by the code scanning through the parameters
in configuration.cc.

Update the look_path function to first try a file without consulting
the PATH environment variable.

Add the find_executable function, and update the code to check the file
is not a directory instead of checking if it is a regular file, so that
symlinks are handled correctly.

Update the peek_file function to do the same, so that ParseManager can
handle symlinks correctly.

The old code worked because the last argument (before the executable
arguments) was update correctly thanks to the loop over the PATH
environment variable.

Note that the correct solution is to add "--" before the executable
arguments, but this will break the kcov cli interface.
  • Loading branch information
perillo committed Mar 10, 2024
1 parent 3015693 commit b587483
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 16 deletions.
6 changes: 6 additions & 0 deletions src/include/utils.hh
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,12 @@ static inline uint32_t hash_block(const void *buf, size_t len)

uint32_t hash_file(const std::string &filename);

int find_executable(const std::string &file);

// Searches for an executable named file in the directories named by the PATH
// environment variable.
// The named file is first tried directly without consulting PATH.
//
// Don't use this function when expanding the first argument of the exec()
// family of functions, since it is insecure.
int look_path(const std::string &file, std::string *out);
51 changes: 35 additions & 16 deletions src/utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -146,8 +146,9 @@ void *peek_file(size_t *out_size, const char *fmt, ...)
struct stat st;
if (lstat(path, &st) < 0)
return NULL;
// Regular file?
if (S_ISREG(st.st_mode) == 0)

// Not a directory?
if (S_ISDIR(st.st_mode) > 0)
return NULL;

// Read a little bit of the file
Expand Down Expand Up @@ -776,8 +777,35 @@ uint32_t hash_file(const std::string &filename)
return out;
}

int find_executable(const std::string &file)
{
struct stat st;

if (lstat(file.c_str(), &st) < 0)
return -1;

// Not a directory?
if (S_ISDIR(st.st_mode) > 0)
return -1;

// Executable?
if ((st.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH)) == 0)
return -1;

return 0;
}


int look_path(const std::string &file, std::string *out)
{
// First check if the relative file is an executable.
if (find_executable(file) == 0)
{
*out = file;

return 0;
}

const char *sys_path = getenv("PATH");
if (!sys_path)
return -1;
Expand All @@ -788,22 +816,13 @@ int look_path(const std::string &file, std::string *out)
{
const std::string root = *it;
const std::string path = get_real_path(root + "/" + file);
struct stat st;

if (lstat(path.c_str(), &st) < 0)
continue;

// Regular file?
if (S_ISREG(st.st_mode) == 0)
continue;

// Executable?
if ((st.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH)) == 0)
continue;

*out = path;
if (find_executable(path) == 0)
{
*out = path;

return 0;
return 0;
}
}

return -1;
Expand Down

0 comments on commit b587483

Please sign in to comment.