Re: [PATCH ima-evm-utils 3/4] Sanity check the template data field sizes

[Stefan Berger <stefanb@xxxxxxxxxxxxx>]

On 9/14/22 10:22, Mimi Zohar wrote:
> The field sizes of the original "ima" template data are static, but
> the other template data fields are not.  They're prefixed with a size.
>
> Add some data field size sanity checks in ima_show_ng().
>
> Signed-off-by: Mimi Zohar <zohar@xxxxxxxxxxxxx>
> ---
>   src/evmctl.c | 42 ++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/src/evmctl.c b/src/evmctl.c
> index 9ab804fee37a..4a071143679e 100644
> --- a/src/evmctl.c
> +++ b/src/evmctl.c
> @@ -1591,8 +1591,9 @@ void ima_ng_show(struct template_entry *entry)
>   {
>   	uint8_t *fieldp = entry->template;
>   	uint32_t field_len;
> -	int total_len = entry->template_len, digest_len, len, sig_len, fbuf_len;
> +	int total_len = entry->template_len, digest_len, len, fbuf_len;
>   	uint8_t *digest, *sig = NULL, *fbuf = NULL;
> +	int sig_len = 0;
>   	char *algo, *path;
>   	int found;
>   	int err;
> @@ -1601,33 +1602,65 @@ void ima_ng_show(struct template_entry *entry)
>   	field_len = *(uint32_t *)fieldp;
>   	fieldp += sizeof(field_len);
>   	total_len -= sizeof(field_len);
> +	if (total_len < 0) {
> +		log_err("Template \"%s\" invalid template data\n", entry->name);
> +		return;
> +	}
>   
>   	algo = (char *)fieldp;
>   	len = strnlen(algo, field_len - 1) + 1;
>   	digest_len = field_len - len;
> +	if (digest_len < SHA_DIGEST_LENGTH ||
> +	    digest_len > MAX_DIGEST_SIZE) {
> +		log_err("Template \"%s\" invalid digest length\n", entry->name);
> +		return;
> +	}
>   	digest = fieldp + len;
>   
>   	/* move to next field */
>   	fieldp += field_len;
>   	total_len -= field_len;
> +	if (total_len < 0) {
> +		log_err("Template \"%s\" invalid template data\n", entry->name);
> +		return;
> +	}
>   
>   	/* get path */
>   	field_len = *(uint32_t *)fieldp;
>   	fieldp += sizeof(field_len);
>   	total_len -= sizeof(field_len);
> +	if (field_len == 0 || field_len > PATH_MAX || total_len < field_len) {
> +		log_err("Template \"%s\" invalid file pathname\n", entry->name);
> +		return;
> +	}
>   
>   	path = (char *)fieldp;
>   
>   	/* move to next field */
>   	fieldp += field_len;
>   	total_len -= field_len;
> +	if (total_len < 0) {
> +		log_err("Template \"%s\" invalid template data\n", entry->name);
> +		return;
> +	}
>   
>   	if (!strcmp(entry->name, "ima-sig") ||
>   	    !strcmp(entry->name, "ima-sigv2")) {
> -		/* get signature */
> +		/* get signature, if it exists */
>   		field_len = *(uint32_t *)fieldp;
>   		fieldp += sizeof(field_len);
> +		if (field_len > MAX_SIGNATURE_SIZE) {
> +			log_err("Template \"%s\" invalid file signature size\n",
> +				entry->name);
> +			return;
> +		}
> +
>   		total_len -= sizeof(field_len);
> +		if (total_len < 0) {
> +			log_err("Template \"%s\" invalid template data\n",
> +				entry->name);
> +			return;
> +		}
>   
>   		if (field_len) {
>   			sig = fieldp;
> @@ -1651,6 +1684,11 @@ void ima_ng_show(struct template_entry *entry)
>   		}
>   	}
>   
> +	if (total_len < 0) {
> +		log_err("Template \"%s\" invalid template data\n", entry->name);
> +		return;
> +	}
> +
>   	/* ascii_runtime_measurements */
>   	if (imaevm_params.verbose > LOG_INFO) {
>   		log_info("%d ", entry->header.pcr);

Reviewed-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>