Skip to content
Permalink
Browse files Browse the repository at this point in the history
a-a-i-d-t-a-cache: sanitize arguments
Parse command lines arguments and use them to create new arguments for
exec(). No black listing algorithm would be safe enough. The only
allowed arguments are the following:
* v - verbose
* y - noninteractive
* repo - enable only repositories whose names match the pattern
* exact - download packages for the specified files
* ids - passed as magic proc fd path to the wrapped executable

The wrapper opens the list of needed build ids passes /proc/self/fd/[fd]
to the wrapped process. This allows us to open the file with caller's
UID/GID in order to avoid information disclosures.

Forbidden arguments:
* cache - allows regular users to create a user writable dump directory
* tmpdir - the same as above
* size_mb - no need to allow users to fiddle with the cache size

Related: #1216962

Signed-off-by: Jakub Filak <jfilak@redhat.com>
  • Loading branch information
Jakub Filak committed May 4, 2015
1 parent f3c2a6a commit 9943a77
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 22 deletions.
1 change: 1 addition & 0 deletions po/POTFILES.in
Expand Up @@ -31,6 +31,7 @@ src/plugins/abrt-action-check-oops-for-hw-error.in
src/plugins/abrt-action-generate-backtrace.c
src/plugins/abrt-action-generate-core-backtrace.c
src/plugins/abrt-action-install-debuginfo.in
src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c
src/plugins/abrt-action-perform-ccpp-analysis.in
src/plugins/abrt-action-trim-files.c
src/plugins/abrt-action-ureport
Expand Down
106 changes: 84 additions & 22 deletions src/plugins/abrt-action-install-debuginfo-to-abrt-cache.c
Expand Up @@ -29,28 +29,90 @@
*/
int main(int argc, char **argv)
{
/*
* We disallow passing of arguments which point to writable dirs
* and other files possibly not accessible to calling user.
* This way, the script will always use default values for these arguments.
*/
char **pp = argv;
char *arg;
while ((arg = *++pp) != NULL)
/* I18n */
setlocale(LC_ALL, "");
#if ENABLE_NLS
bindtextdomain(PACKAGE, LOCALEDIR);
textdomain(PACKAGE);
#endif

abrt_init(argv);

/* Can't keep these strings/structs static: _() doesn't support that */
const char *program_usage_string = _(
"& [-y] [-i BUILD_IDS_FILE|-i -] [-e PATH[:PATH]...]\n"
"\t[-r REPO]\n"
"\n"
"Installs debuginfo packages for all build-ids listed in BUILD_IDS_FILE to\n"
"ABRT system cache."
);

enum {
OPT_v = 1 << 0,
OPT_y = 1 << 1,
OPT_i = 1 << 2,
OPT_e = 1 << 3,
OPT_r = 1 << 4,
OPT_s = 1 << 5,
};

const char *build_ids = "build_ids";
const char *exact = NULL;
const char *repo = NULL;
const char *size_mb = NULL;

struct options program_options[] = {
OPT__VERBOSE(&g_verbose),
OPT_BOOL ('y', "yes", NULL, _("Noninteractive, assume 'Yes' to all questions")),
OPT_STRING('i', "ids", &build_ids, "BUILD_IDS_FILE", _("- means STDIN, default: build_ids")),
OPT_STRING('e', "exact", &exact, "EXACT", _("Download only specified files")),
OPT_STRING('r', "repo", &repo, "REPO", _("Pattern to use when searching for repos, default: *debug*")),
OPT_STRING('s', "size_mb", &size_mb, "SIZE_MB", _("Ignored option")),
OPT_END()
};
const unsigned opts = parse_opts(argc, argv, program_options, program_usage_string);

/* We need to open the build ids file under the caller's UID/GID to avoid
* information disclosures when reading files with changed UID.
* Unfortunately, we cannot replace STDIN with the new fd because ABRT uses
* STDIN to communicate with the caller. So, the following code opens a
* dummy file descriptor to the build ids file and passes the new fd's proc
* path to the wrapped program in the ids argument.
* The new fd remains opened, the OS will close it for us. */
char *build_ids_self_fd = NULL;
if (strcmp("-", build_ids) != 0)
{
const int build_ids_fd = open(build_ids, O_RDONLY);
if (build_ids_fd < 0)
perror_msg_and_die("Failed to open file '%s'", build_ids);

/* We are not going to free this memory. There is no place to do so. */
build_ids_self_fd = xasprintf("/proc/self/fd/%d", build_ids_fd);
}

/* name, -v, --ids, -, -y, -e, EXACT, -r, REPO, --, NULL */
const char *args[11];
{
/* Allow taking ids from stdin */
if (strcmp(arg, "--ids=-") == 0)
continue;

if (strncmp(arg, "--exact", 7) == 0)
continue;

if (strncmp(arg, "--cache", 7) == 0)
error_msg_and_die("bad option %s", arg);
if (strncmp(arg, "--tmpdir", 8) == 0)
error_msg_and_die("bad option %s", arg);
if (strncmp(arg, "--ids", 5) == 0)
error_msg_and_die("bad option %s", arg);
const char *verbs[] = { "", "-v", "-vv", "-vvv" };
unsigned i = 0;
args[i++] = EXECUTABLE;
args[i++] = "--ids";
args[i++] = (build_ids_self_fd != NULL) ? build_ids_self_fd : "-";
args[i++] = verbs[g_verbose <= 3 ? g_verbose : 3];
if ((opts & OPT_y))
args[i++] = "-y";
if ((opts & OPT_e))
{
args[i++] = "--exact";
args[i++] = exact;
}
if ((opts & OPT_r))
{
args[i++] = "--repo";
args[i++] = repo;
}
args[i++] = "--";
args[i] = NULL;
}

/* Switch real user/group to effective ones.
Expand Down Expand Up @@ -122,6 +184,6 @@ int main(int argc, char **argv)
putenv(path_env);
}

execvp(EXECUTABLE, argv);
execvp(EXECUTABLE, (char **)args);
error_msg_and_die("Can't execute %s", EXECUTABLE);
}

0 comments on commit 9943a77

Please sign in to comment.