[pacman-dev] [PATCH] Fix out of boundary reads in pacsort.

[Andrew Gregory]

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 <tobias at 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