Re: [PATCH] conf-parse.y checks (with style fixes removed)

[Stephen Smalley <sds@xxxxxxxxxxxxx>]

On 5/18/19 12:57 PM, Jokke Hämäläinen wrote:
> diff --git a/libsemanage/src/conf-parse.y b/libsemanage/src/conf-parse.y
> index b527e893..64b53c2d 100644
> --- a/libsemanage/src/conf-parse.y
> +++ b/libsemanage/src/conf-parse.y
> @@ -221,6 +221,9 @@ usepasswd: USEPASSWD '=' ARG {
>   
>   ignoredirs: IGNOREDIRS '=' ARG {
>   	current_conf->ignoredirs = strdup($3);
> +	if (current_conf->ignoredirs == NULL) {
> +		yyerror("could not allocate memory for current_conf->ignoredirs");

I wouldn't put the variable name/field name into the error message. 
"could not allocate memory for ignoredirs" would suffice.

> +	}
>   	free($3);
>    }
>   
> @@ -240,8 +243,8 @@ handle_unknown: HANDLE_UNKNOWN '=' ARG {
>   bzip_blocksize:  BZIP_BLOCKSIZE '=' ARG {
>   	int blocksize = atoi($3);
>   	free($3);
> -	if (blocksize > 9)
> -		yyerror("bzip-blocksize can only be in the range 0-9");
> +	if (blocksize < 1 || blocksize > 9)
> +		yyerror("bzip-blocksize can only be in the range 1-9");
>   	else
>   		current_conf->bzip_blocksize = blocksize;
>   }
> @@ -339,9 +342,18 @@ static int semanage_conf_init(semanage_conf_t * conf)
>   {
>   	conf->store_type = SEMANAGE_CON_DIRECT;
>   	conf->store_path = strdup(basename(selinux_policy_root()));
> +	if (conf->store_path == NULL) {
> +		return -1;
> +	}
>   	conf->ignoredirs = NULL;
>   	conf->store_root_path = strdup("/var/lib/selinux");
> +	if (conf->store_root_path == NULL) {
> +		return -1;
> +	}
>   	conf->compiler_directory_path = strdup("/usr/libexec/selinux/hll");
> +	if (conf->compiler_directory_path == NULL) {
> +		return -1;
> +	}
>   	conf->policyvers = sepol_policy_kern_vers_max();
>   	conf->target_platform = SEPOL_TARGET_SELINUX;
>   	conf->expand_check = 1;
> @@ -515,7 +527,10 @@ static int parse_module_store(char *arg)
>   			}
>   		}
>   	}
> -	return 0;
> +
> +	if (current_conf->store_path)
> +		return 0;
> +	return -3;
>   }
>   
>   static int parse_store_root_path(char *arg)
> @@ -526,6 +541,10 @@ static int parse_store_root_path(char *arg)
>   
>   	free(current_conf->store_root_path);
>   	current_conf->store_root_path = strdup(arg);
> +	if (current_conf->store_root_path == NULL) {
> +		return -2;

Not sure why we are using multiple error return codes here or what they 
mean.

> +	}
> +
>   	return 0;
>   }
>   
> @@ -536,6 +555,9 @@ static int parse_compiler_path(char *arg)
>   	}
>   	free(current_conf->compiler_directory_path);
>   	current_conf->compiler_directory_path = strdup(arg);
> +	if (current_conf->compiler_directory_path = NULL) {
> +		return -2;
> +	}
>   	return 0;
>   }
>