On 09/22/18 at 11:30pm, morganamilo wrote:
in the function query_fileowner, if the user enters a string longer than PATH_MAX then rpath will buffer overflow when lrealpath tries to strcat everything together.
So make sure to bail early if the generated path is going to be bigger than PATH_MAX.
This also fixes the compiler warning: query.c: In function ‘query_fileowner’: query.c:192:4: warning: ‘strncpy’ specified bound 4096 equals destination size [-Wstringop-truncation] strncpy(rpath, filename, PATH_MAX);
Signed-off-by: morganamilo
diff --git a/src/pacman/query.c b/src/pacman/query.c index 00c39638..c661fafb 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -102,7 +102,7 @@ static char *lrealpath(const char *path, char *resolved_path) { const char *bname = mbasename(path); char *rpath = NULL, *dname = NULL; - int success = 0; + int success = 0, len;
if(strcmp(bname, ".") == 0 || strcmp(bname, "..") == 0) { /* the entire path needs to be resolved */ @@ -115,8 +115,14 @@ static char *lrealpath(const char *path, char *resolved_path) if(!(rpath = realpath(dname, NULL))) { goto cleanup; } + + len = strlen(rpath) + strlen(bname) + 2; + if (len > PATH_MAX) { + errno = ENAMETOOLONG; + goto cleanup; + } if(!resolved_path) { - if(!(resolved_path = malloc(strlen(rpath) + strlen(bname) + 2))) { + if(!(resolved_path = malloc(len))) { goto cleanup; } } @@ -187,11 +193,16 @@ static int query_fileowner(alpm_list_t *targets) } }
+ errno = 0;
No need to explicitly set errno here. lrealpath should always set it on failure.
if(!lrealpath(filename, rpath)) { + if (errno == ENAMETOOLONG) { + pm_printf(ALPM_LOG_ERROR, _("path too long: %s/\n"), filename); + goto targcleanup; + } /* Can't canonicalize path, try to proceed anyway */ - strncpy(rpath, filename, PATH_MAX); + strncpy(rpath, filename, PATH_MAX - 1); + rpath[PATH_MAX - 1] = '\0';
This silences the warning, but filename could still be longer than PATH_MAX, in which case the results will be incorrect. I think the best thing to do is treat ENAMETOOLONG the same as any other lrealpath error, and explicitly check if strlen(filename) >= PATH_MAX instead.
} -
Please leave this empty line to separate the two blocks.
if(strncmp(rpath, root, rootlen) != 0) { /* file is outside root, we know nothing can own it */ pm_printf(ALPM_LOG_ERROR, _("No package owns %s\n"), filename); -- 2.19.0