Re: [libnftnl PATCH] ruleset: crash in path error when we build the xml tree

[Pablo Neira Ayuso <pablo@netfilter.org>]

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

On Tue, Feb 24, 2015 at 09:10:33AM +0100, Alvaro Neira Ayuso wrote:
> Crash when we try to release a tree that is not initialized.
> 
> Signed-off-by: Alvaro Neira Ayuso <alvaroneay@gmail.com>
> ---
>  src/ruleset.c |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ruleset.c b/src/ruleset.c
> index 9e8965c..8549130 100644
> --- a/src/ruleset.c
> +++ b/src/ruleset.c
> @@ -669,8 +669,10 @@ static int nft_ruleset_xml_parse(const void *xml, struct nft_parse_err *err,
>  		nft_ruleset_ctx_set(&ctx, NFT_RULESET_CTX_DATA, arg);
>  
>  	tree = nft_mxml_build_tree(xml, "nftables", err, input);
> -	if (tree == NULL)
> -		goto err;
> +	if (tree == NULL) {
> +		nft_set_list_free(ctx.set_list);
> +		return -1;
> +	}

You have exactly the same problem in nft_ruleset_json_parse().

Have a look at the patch attached, it provides a template on how to
fix this.

Another different thing: it would be good to use the new 'buffer'
class to snprintf the ruleset.

[-- Attachment #2: x.patch --]
[-- Type: text/x-diff, Size: 863 bytes --]

diff --git a/src/ruleset.c b/src/ruleset.c
index 89ea344..86d8033 100644
--- a/src/ruleset.c
+++ b/src/ruleset.c
@@ -665,7 +665,7 @@ static int nft_ruleset_xml_parse(const void *xml, struct nft_parse_err *err,
 
 	tree = nft_mxml_build_tree(xml, "nftables", err, input);
 	if (tree == NULL)
-		goto err;
+		goto err1;
 
 	ctx.xml = tree;
 
@@ -673,16 +673,17 @@ static int nft_ruleset_xml_parse(const void *xml, struct nft_parse_err *err,
 	while (nodecmd != NULL) {
 		cmd = nodecmd->value.opaque;
 		if (nft_ruleset_xml_parse_cmd(cmd, err, &ctx) < 0)
-			goto err;
+			goto err2;
 		nodecmd = mxmlWalkNext(tree, tree, MXML_NO_DESCEND);
 	}
 
 	nft_set_list_free(ctx.set_list);
 	mxmlDelete(tree);
 	return 0;
-err:
-	nft_set_list_free(ctx.set_list);
+err2:
 	mxmlDelete(tree);
+err1:
+	nft_set_list_free(ctx.set_list);
 	return -1;
 #else
 	errno = EOPNOTSUPP;