On 06/09/16 at 11:16pm, Tobias Stoeckmann wrote: > This patch fixes two out of bound reads in pacsort (as they are of the > same error type, I didn't see a reason to split it into two commits). > > The first one is triggered by a very large "key column" and the > delimiter '\0'. The return value of strchr() is not checked for the > special case in which it reaches the end of the version string. > I recommend to simply not allow the '\0' separator, because file names > cannot contain it. And therefore, it cannot be a sane column separator > in file names for versions either.
-t and -k are used to sort tabular data, where NUL is a perfectly sane field separator. In fact, with --machinereadable pacman will output a table of NUL separated fields. Though, pacsort does need its handling of -t '\0' fixed in other places as well. > You can trigger it by running this command: > > $ echo -e "-.pkg.tar.xz\n-.pkg.tar.xz" | pacsort -k 999999 -t '\0' > Segmentation fault > > The second one is triggered due to a wrong assumption in fnmatch > pattern. It is assumed that "*-*.pkg.tar.?z" will only match file names > which contain a dash. But in fact, it also accepts files without one, > if at least one of the directories contains it. One example would be > "/my-path/file.pkg.tar.xz". > > While parsing the version string, the return value of strrchr() looking > for the dash is not checked anymore. In this case, it would return NULL > and further pointer arithmetic on it turns it into SIZE_MAX. Eventually, > pacman segfaults while traversing this memory area for needed chars: > > $ echo "-/.pkg.tar.xz" | pacsort > Segmentation fault > > Signed-off-by: Tobias Stoeckmann <tob...@stoeckmann.org> > --- > src/util/pacsort.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/src/util/pacsort.c b/src/util/pacsort.c > index 662b250..65f67bd 100644 > --- a/src/util/pacsort.c > +++ b/src/util/pacsort.c > @@ -228,6 +228,9 @@ static struct input_t *input_new(const char *path, int > pathlen) > } > > pkgver_end = strrchr(in->pkgname, '-'); > + if(pkgver_end == NULL || pkgver_end == in->pkgname) { > + return in; > + } > > /* read backwards through pkgrel */ > for(in->pkgver = pkgver_end - 1; > @@ -410,8 +413,6 @@ static char escape_char(const char *string) > return '\n'; > case 'v': > return '\v'; > - case '0': > - return '\0'; > default: > return INVALD_ESCAPE_CHAR; > } > -- > 2.8.3