| Message ID | 20180922201645.13413-2-morganamilo@gmail.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | Andrew Gregory |
| Headers | show |
| Series | [pacman-dev,1/2] pacman: fix possible buffer overflow | expand |
On 09/22/18 at 09:16pm, morganamilo wrote: > The string only becomes longer than PATH_MAX once adding "/" to the end. > The error message should give this version of the path. > > Signed-off-by: morganamilo <morganamilo@gmail.com> NACK. The trailing slash is in the format string; your version now has two trailing slashes.. > diff --git a/src/pacman/query.c b/src/pacman/query.c > index a1197cea..ecf8d148 100644 > --- a/src/pacman/query.c > +++ b/src/pacman/query.c > @@ -207,15 +207,15 @@ static int query_fileowner(alpm_list_t *targets) > rel_path = rpath + rootlen; > > if((is_missing && is_dir) || (!is_missing && (is_dir = S_ISDIR(buf.st_mode)))) { > - if(rlen + 2 >= PATH_MAX) { > - pm_printf(ALPM_LOG_ERROR, _("path too long: %s/\n"), rpath); > - goto targcleanup; > - } > if ((rpathsave = realloc(rpath, rlen + 2)) == NULL) { > goto targcleanup; > } > rpath = rpathsave; > strcat(rpath + rlen, "/"); > + if(rlen + 2 >= PATH_MAX) { > + pm_printf(ALPM_LOG_ERROR, _("path too long: %s/\n"), rpath); > + goto targcleanup; > + } > } > > for(i = packages; i && (!found || is_dir); i = alpm_list_next(i)) { > -- > 2.19.0
On Sat, 22 Sep 2018 at 21:54, Andrew Gregory <andrew.gregory.8@gmail.com> wrote: > > On 09/22/18 at 09:16pm, morganamilo wrote: > > The string only becomes longer than PATH_MAX once adding "/" to the end. > > The error message should give this version of the path. > > > > Signed-off-by: morganamilo <morganamilo@gmail.com> > > NACK. The trailing slash is in the format string; your version now > has two trailing slashes.. > > > diff --git a/src/pacman/query.c b/src/pacman/query.c > > index a1197cea..ecf8d148 100644 > > --- a/src/pacman/query.c > > +++ b/src/pacman/query.c > > @@ -207,15 +207,15 @@ static int query_fileowner(alpm_list_t *targets) > > rel_path = rpath + rootlen; > > > > if((is_missing && is_dir) || (!is_missing && (is_dir = S_ISDIR(buf.st_mode)))) { > > - if(rlen + 2 >= PATH_MAX) { > > - pm_printf(ALPM_LOG_ERROR, _("path too long: %s/\n"), rpath); > > - goto targcleanup; > > - } > > if ((rpathsave = realloc(rpath, rlen + 2)) == NULL) { > > goto targcleanup; > > } > > rpath = rpathsave; > > strcat(rpath + rlen, "/"); > > + if(rlen + 2 >= PATH_MAX) { > > + pm_printf(ALPM_LOG_ERROR, _("path too long: %s/\n"), rpath); > > + goto targcleanup; > > + } > > } > > > > for(i = packages; i && (!found || is_dir); i = alpm_list_next(i)) { > > -- > > 2.19.0 Oh yeah silly me, missed the / inside the printf. Disregard this patch then.
diff --git a/src/pacman/query.c b/src/pacman/query.c index a1197cea..ecf8d148 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -207,15 +207,15 @@ static int query_fileowner(alpm_list_t *targets) rel_path = rpath + rootlen; if((is_missing && is_dir) || (!is_missing && (is_dir = S_ISDIR(buf.st_mode)))) { - if(rlen + 2 >= PATH_MAX) { - pm_printf(ALPM_LOG_ERROR, _("path too long: %s/\n"), rpath); - goto targcleanup; - } if ((rpathsave = realloc(rpath, rlen + 2)) == NULL) { goto targcleanup; } rpath = rpathsave; strcat(rpath + rlen, "/"); + if(rlen + 2 >= PATH_MAX) { + pm_printf(ALPM_LOG_ERROR, _("path too long: %s/\n"), rpath); + goto targcleanup; + } } for(i = packages; i && (!found || is_dir); i = alpm_list_next(i)) {
The string only becomes longer than PATH_MAX once adding "/" to the end. The error message should give this version of the path. Signed-off-by: morganamilo <morganamilo@gmail.com>