[pacman-dev] [PATCH] Fix out of boundary reads in pacsort.
andrew.gregory.8 at gmail.com
Fri Jun 10 23:11:16 UTC 2016
On 06/11/16 at 12:14am, Tobias Stoeckmann wrote:
> On Thu, Jun 09, 2016 at 07:46:00PM -0400, Andrew Gregory wrote:
> > -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.
> The function escape_char is only called from main for the 't' option
> argument. The result is stored in opts.delim.
> The value of opts.delim is only used in function nth_column. That
> function is called in compare_versions, which is called by vercmp
> either directly or through compare_files.
> And that one is called by main again. Which "other places" do you mean
> that have to be fixed? There is only one.
> The parsing of input happens in input_new, which allocates space for
> in->data by using strndup(). Whatever points into this new data area,
> accessing past the first '\0' is a straight out of boundary access.
> No matter the origin of pacsort's input, first '\0' definitely means
> 'end here'.
> Maybe you mixed up the stand alone utility "pacsort" with some
> function in libalpm?
> Otherwise please let me know what exactly is broken now by showing the
> code path or actual program invocation.
I was saying that your rationale for just dropping support for -t '\0'
was incorrect. NUL not being legal in a path name is irrelevant.
But, as you just pointed out, pacsort has more issues with -t '\0'
than just the poor use of strchr referenced by your patch. Hence,
"pacsort does need its handling of -t '\0' fixed in other places as
well." In other words, if -t '\0' is kept, pacsort needs to be fixed
in a few places.
More information about the pacman-dev