"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Tracey Emery <tracey@traceyemery.net>
Subject:
Re: fix object header parser for zero-length object headers
To:
gameoftrees@openbsd.org
Date:
Fri, 28 Jan 2022 08:00:04 -0700

Download raw body.

Thread
On Fri, Jan 28, 2022 at 01:02:28PM +0100, Stefan Sperling wrote:
> By code inspection I found that got_object_read_header() will pass
> non-NUL-terminated strings into strnlen() if an object file is empty.
> 
> The patch below tidies up relevant code, makes the parser more robust,
> and takes potential empty input into account.
> 
> The header is simple, a tag string which identifies the object type
> such as "commit ", followed by the object size encoded as an ASCII
> number string, followed by \0.
> 
> If you want to see an example, just decompress any object from one of the
> directories "objects/[0-9a-f][0-9a-f]/" in any repository, using the
> script util/uncompress-loose-object.sh from the got source tree.
> You need to run 'pkg_add qpdf' to install zlib-flate which is used by
> this script.
> 
> ok?

ok

>  
> diff 8669fd5c8f1a790cfe005834fd9fbd34ceef71f6 be9053848281194022cfb726e21a361af43cadba
> blob - b4f415940ee578ecafa1e8ceca60032674013904
> blob + 799a16dd7e542f69c5669859576f374888904a16
> --- lib/object_parse.c
> +++ lib/object_parse.c
> @@ -196,13 +196,12 @@ got_object_parse_header(struct got_object **obj, char 
>  		GOT_OBJ_TYPE_TAG,
>  	};
>  	int type = 0;
> -	size_t size = 0, hdrlen = 0;
> +	size_t size = 0;
>  	size_t i;
>  
>  	*obj = NULL;
>  
> -	hdrlen = strnlen(buf, len) + 1 /* '\0' */;
> -	if (hdrlen > len)
> +	if (memchr(buf, '\0', len) == NULL)
>  		return got_error(GOT_ERR_BAD_OBJ_HDR);
>  
>  	for (i = 0; i < nitems(obj_labels); i++) {
> @@ -210,12 +209,10 @@ got_object_parse_header(struct got_object **obj, char 
>  		size_t label_len = strlen(label);
>  		const char *errstr;
>  
> -		if (strncmp(buf, label, label_len) != 0)
> +		if (len <= label_len || strncmp(buf, label, label_len) != 0)
>  			continue;
>  
>  		type = obj_types[i];
> -		if (len <= label_len)
> -			return got_error(GOT_ERR_BAD_OBJ_HDR);
>  		size = strtonum(buf + label_len, 0, LONG_MAX, &errstr);
>  		if (errstr != NULL)
>  			return got_error(GOT_ERR_BAD_OBJ_HDR);
> @@ -229,7 +226,7 @@ got_object_parse_header(struct got_object **obj, char 
>  	if (*obj == NULL)
>  		return got_error_from_errno("calloc");
>  	(*obj)->type = type;
> -	(*obj)->hdrlen = hdrlen;
> +	(*obj)->hdrlen = strnlen(buf, len) + 1 /* '\0' */;
>  	(*obj)->size = size;
>  	return NULL;
>  }
> @@ -249,6 +246,7 @@ got_object_read_header(struct got_object **obj, int fd
>  	buf = malloc(zbsize);
>  	if (buf == NULL)
>  		return got_error_from_errno("malloc");
> +	buf[0] = '\0';
>  
>  	err = got_inflate_init(&zb, buf, zbsize, NULL);
>  	if (err)
> 

-- 

Tracey Emery