[pacman-dev] [PATCH] Do not add root prefix twice when checking database files

[Andrew Gregory]

On 03/20/16 at 09:38pm, Allan McRae wrote:
> When checking .INSTALL and .CHANGELOG files in the mtree file, we need to find
> the path they are stored in the local database. This was appending the root
> prefix twice as alpm_option_get_dbpath already includes the root path.
> 
> While fixing that issue I added checks that the paths for the database files
> were not longer than PATH_MAX.
> 
> Fixes FS#48563.
> 
> Signed-off-by: Allan McRae <allan at archlinux.org>
> ---
> 
> For backport to 5.0.2.
> 
>  src/pacman/check.c | 58 ++++++++++++++++++++++++++++++++----------------------
>  1 file changed, 34 insertions(+), 24 deletions(-)
> 
> diff --git a/src/pacman/check.c b/src/pacman/check.c
> index 0fe74e8..86c25c8 100644
> --- a/src/pacman/check.c
> +++ b/src/pacman/check.c
> @@ -254,7 +254,6 @@ int check_pkg_full(alpm_pkg_t *pkg)
>  	const char *root, *pkgname;
>  	size_t errors = 0;
>  	size_t rootlen;
> -	char filepath[PATH_MAX];
>  	struct archive *mtree;
>  	struct archive_entry *entry = NULL;
>  	size_t file_count = 0;
> @@ -267,7 +266,6 @@ int check_pkg_full(alpm_pkg_t *pkg)
>  		pm_printf(ALPM_LOG_ERROR, _("path too long: %s%s\n"), root, "");
>  		return 1;
>  	}
> -	strcpy(filepath, root);
>  
>  	pkgname = alpm_pkg_get_name(pkg);
>  	mtree = alpm_pkg_mtree_open(pkg);
> @@ -282,6 +280,8 @@ int check_pkg_full(alpm_pkg_t *pkg)
>  	while(alpm_pkg_mtree_next(pkg, mtree, &entry) == ARCHIVE_OK) {
>  		struct stat st;
>  		const char *path = archive_entry_pathname(entry);
> +		char filepath[PATH_MAX];
> +		int filepath_len;
>  		mode_t type;
>  		size_t file_errors = 0;
>  		int backup = 0;
> @@ -292,32 +292,42 @@ int check_pkg_full(alpm_pkg_t *pkg)
>  			path += 2;
>  		}
>  
> -		if(strcmp(path, ".INSTALL") == 0) {
> -			char filename[PATH_MAX];
> -			snprintf(filename, PATH_MAX, "%slocal/%s-%s/install",
> -					alpm_option_get_dbpath(config->handle) + 1,
> -					pkgname, alpm_pkg_get_version(pkg));
> -			archive_entry_set_pathname(entry, filename);
> -			path = archive_entry_pathname(entry);
> -		} else if(strcmp(path, ".CHANGELOG") == 0) {
> -			char filename[PATH_MAX];
> -			snprintf(filename, PATH_MAX, "%slocal/%s-%s/changelog",
> -					alpm_option_get_dbpath(config->handle) + 1,
> -					pkgname, alpm_pkg_get_version(pkg));
> -			archive_entry_set_pathname(entry, filename);
> -			path = archive_entry_pathname(entry);
> -		} else if(*path == '.') {
> -			continue;
> +		if(*path == '.') {
> +			if(strcmp(path, ".INSTALL") == 0) {
> +				char filename[PATH_MAX];
> +				snprintf(filename, PATH_MAX, "%slocal/%s-%s/install",
> +						alpm_option_get_dbpath(config->handle) + 1,
> +						pkgname, alpm_pkg_get_version(pkg));
> +				archive_entry_set_pathname(entry, filename);
> +				path = archive_entry_pathname(entry);
> +			} else if(strcmp(path, ".CHANGELOG") == 0) {
> +				char filename[PATH_MAX];
> +				snprintf(filename, PATH_MAX, "%slocal/%s-%s/changelog",
> +						alpm_option_get_dbpath(config->handle) + 1,
> +						pkgname, alpm_pkg_get_version(pkg));
> +				archive_entry_set_pathname(entry, filename);
> +				path = archive_entry_pathname(entry);
> +			} else {
> +				continue;
> +			}
> +
> +			/* Do not append root directory as alpm_option_get_dbpath already
> +			 * includes it */

This is not necessarily correct.  dbpath can be *outside* of our root;
the important thing is that dbpath is already absolute.

> +			filepath_len = snprintf(filepath, PATH_MAX, "/%s", archive_entry_pathname(entry));

You skip the leading '/' above only to add it back here, and the above
snprintf has no overflow check.  I would suggest doing something
similar to extract_db_file in add.c and have the above if/else chain
just set the name of the dbfile and do all of the actual path
manipulation here.

> +			if(filepath_len >= PATH_MAX) {
> +				pm_printf(ALPM_LOG_WARNING, _("path too long: %s%s\n"), root, path);
> +				continue;
> +			}
> +		} else {
> +			filepath_len = snprintf(filepath, PATH_MAX, "%s%s", root, path);
> +			if(filepath_len >= PATH_MAX) {
> +				pm_printf(ALPM_LOG_WARNING, _("path too long: %s%s\n"), root, path);
> +				continue;
> +			}
>  		}
>  
>  		file_count++;
>  
> -		if(rootlen + 1 + strlen(path) > PATH_MAX) {
> -			pm_printf(ALPM_LOG_WARNING, _("path too long: %s%s\n"), root, path);
> -			continue;
> -		}
> -		strcpy(filepath + rootlen, path);
> -
>  		exists = check_file_exists(pkgname, filepath, rootlen, &st);
>  		if(exists == 1) {
>  			errors++;
> -- 
> 2.7.3