[pacman-dev] [PATCH] Reject files larger than INT_MAX in read_sigfile.

[Allan McRae]

On 07/06/16 03:59, Tobias Stoeckmann wrote:
> Signature files larger than INT_MAX are already suspicious,
> but if they are larger than SIZE_MAX, this code couldn't even
> copy them into memory, accepting them as "blank" files at worst.
> 
> While adding the INT_MAX check, I also rearranged the code to
> avoid a quite harmless TOCTOU race condition between
> stat() and fopen().
> 
> Signed-off-by: Tobias Stoeckmann <tobias at stoeckmann.org>
> ---
> I don't know about any sane limitation of signature files, so I just
> took INT_MAX. It's an implementation limit of pacman.

In repo-add we have:

if (( pgpsigsize > 16384 )); then
error "$(gettext "Invalid package signature file '%s'.")" "$pkgfile.sig"

So I think we should be consistent here.

Also, INT_MAX is implementation dependant (although I am not sure this
varies in practice...).  We don't want a signature on an arch=any
package to be rejected on some systems and not others.

A solution to both:

#define MAX_SIGFILE_SIZE 16384

Thanks,
Allan


> ---
>  lib/libalpm/be_package.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/libalpm/be_package.c b/lib/libalpm/be_package.c
> index c9ed770..fc455e8 100644
> --- a/lib/libalpm/be_package.c
> +++ b/lib/libalpm/be_package.c
> @@ -24,6 +24,7 @@
>  #include <sys/types.h>
>  #include <sys/stat.h>
>  #include <fcntl.h>
> +#include <limits.h>
>  
>  /* libarchive */
>  #include <archive.h>
> @@ -700,17 +701,17 @@ static int read_sigfile(const char *sigpath, unsigned char **sig)
>  	struct stat st;
>  	FILE *fp;
>  
> -	if(stat(sigpath, &st) != 0) {
> +	if((fp = fopen(sigpath, "rb")) == NULL) {
>  		return -1;
>  	}
>  
> -	MALLOC(*sig, st.st_size, return -1);
> -
> -	if((fp = fopen(sigpath, "rb")) == NULL) {
> -		free(*sig);
> +	if(fstat(fileno(fp), &st) != 0 || st.st_size > INT_MAX) {
> +		fclose(fp);
>  		return -1;
>  	}
>  
> +	MALLOC(*sig, st.st_size, fclose(fp); return -1);
> +
>  	if(fread(*sig, st.st_size, 1, fp) != 1) {
>  		free(*sig);
>  		fclose(fp);
>